Merge "MIPS32: Ensure preservation of RA in leaf methods if it's clobbered"
diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h
index 00a779a..0c60a98 100644
--- a/compiler/optimizing/code_generator.h
+++ b/compiler/optimizing/code_generator.h
@@ -624,7 +624,7 @@
return POPCOUNT(core_spill_mask_) * GetWordSize();
}
- bool HasAllocatedCalleeSaveRegisters() const {
+ virtual bool HasAllocatedCalleeSaveRegisters() const {
// We check the core registers against 1 because it always comprises the return PC.
return (POPCOUNT(allocated_registers_.GetCoreRegisters() & core_callee_save_mask_) != 1)
|| (POPCOUNT(allocated_registers_.GetFloatingPointRegisters() & fpu_callee_save_mask_) != 0);
diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc
index b073775..36bb55a 100644
--- a/compiler/optimizing/code_generator_mips.cc
+++ b/compiler/optimizing/code_generator_mips.cc
@@ -666,16 +666,17 @@
if ((fpu_spill_mask_ != 0) && (POPCOUNT(core_spill_mask_) % 2 != 0)) {
core_spill_mask_ |= (1 << ZERO);
}
+}
+
+bool CodeGeneratorMIPS::HasAllocatedCalleeSaveRegisters() const {
// If RA is clobbered by PC-relative operations on R2 and it's the only spilled register
- // (this can happen in leaf methods), artificially spill the ZERO register in order to
- // force explicit saving and restoring of RA. RA isn't saved/restored when it's the only
- // spilled register.
+ // (this can happen in leaf methods), force CodeGenerator::InitializeCodeGeneration()
+ // into the path that creates a stack frame so that RA can be explicitly saved and restored.
+ // RA can't otherwise be saved/restored when it's the only spilled register.
// TODO: Can this be improved? It causes creation of a stack frame (while RA might be
// saved in an unused temporary register) and saving of RA and the current method pointer
// in the frame.
- if (clobbered_ra_ && core_spill_mask_ == (1u << RA) && fpu_spill_mask_ == 0) {
- core_spill_mask_ |= (1 << ZERO);
- }
+ return CodeGenerator::HasAllocatedCalleeSaveRegisters() || clobbered_ra_;
}
static dwarf::Reg DWARFReg(Register reg) {
@@ -698,6 +699,9 @@
}
if (HasEmptyFrame()) {
+ CHECK_EQ(fpu_spill_mask_, 0u);
+ CHECK_EQ(core_spill_mask_, 1u << RA);
+ CHECK(!clobbered_ra_);
return;
}
@@ -731,6 +735,7 @@
}
// Store the current method pointer.
+ // TODO: can we not do this if RequiresCurrentMethod() returns false?
__ StoreToOffset(kStoreWord, kMethodRegisterArgument, SP, kCurrentMethodStackOffset);
}
diff --git a/compiler/optimizing/code_generator_mips.h b/compiler/optimizing/code_generator_mips.h
index 4ce54b6..7ba6c0d 100644
--- a/compiler/optimizing/code_generator_mips.h
+++ b/compiler/optimizing/code_generator_mips.h
@@ -274,6 +274,7 @@
virtual ~CodeGeneratorMIPS() {}
void ComputeSpillMask() OVERRIDE;
+ bool HasAllocatedCalleeSaveRegisters() const OVERRIDE;
void GenerateFrameEntry() OVERRIDE;
void GenerateFrameExit() OVERRIDE;
diff --git a/compiler/optimizing/codegen_test.cc b/compiler/optimizing/codegen_test.cc
index fe6c0a3..d00a786 100644
--- a/compiler/optimizing/codegen_test.cc
+++ b/compiler/optimizing/codegen_test.cc
@@ -991,4 +991,67 @@
}
}
+#ifdef ART_ENABLE_CODEGEN_mips
+TEST_F(CodegenTest, MipsClobberRA) {
+ std::unique_ptr<const MipsInstructionSetFeatures> features_mips(
+ MipsInstructionSetFeatures::FromCppDefines());
+ if (!CanExecute(kMips) || features_mips->IsR6()) {
+ // HMipsComputeBaseMethodAddress and the NAL instruction behind it
+ // should only be generated on non-R6.
+ return;
+ }
+
+ ArenaPool pool;
+ ArenaAllocator allocator(&pool);
+ HGraph* graph = CreateGraph(&allocator);
+
+ HBasicBlock* entry_block = new (&allocator) HBasicBlock(graph);
+ graph->AddBlock(entry_block);
+ graph->SetEntryBlock(entry_block);
+ entry_block->AddInstruction(new (&allocator) HGoto());
+
+ HBasicBlock* block = new (&allocator) HBasicBlock(graph);
+ graph->AddBlock(block);
+
+ HBasicBlock* exit_block = new (&allocator) HBasicBlock(graph);
+ graph->AddBlock(exit_block);
+ graph->SetExitBlock(exit_block);
+ exit_block->AddInstruction(new (&allocator) HExit());
+
+ entry_block->AddSuccessor(block);
+ block->AddSuccessor(exit_block);
+
+ // To simplify matters, don't create PC-relative HLoadClass or HLoadString.
+ // Instead, generate HMipsComputeBaseMethodAddress directly.
+ HMipsComputeBaseMethodAddress* base = new (&allocator) HMipsComputeBaseMethodAddress();
+ block->AddInstruction(base);
+ // HMipsComputeBaseMethodAddress is defined as int, so just make the
+ // compiled method return it.
+ block->AddInstruction(new (&allocator) HReturn(base));
+
+ graph->BuildDominatorTree();
+
+ mips::CodeGeneratorMIPS codegenMIPS(graph, *features_mips.get(), CompilerOptions());
+ // Since there isn't HLoadClass or HLoadString, we need to manually indicate
+ // that RA is clobbered and the method entry code should generate a stack frame
+ // and preserve RA in it. And this is what we're testing here.
+ codegenMIPS.ClobberRA();
+ // Without ClobberRA() the code would be:
+ // nal # Sets RA to point to the jr instruction below
+ // move v0, ra # and the CPU falls into an infinite loop.
+ // jr ra
+ // nop
+ // The expected code is:
+ // addiu sp, sp, -16
+ // sw ra, 12(sp)
+ // sw a0, 0(sp)
+ // nal # Sets RA to point to the lw instruction below.
+ // move v0, ra
+ // lw ra, 12(sp)
+ // jr ra
+ // addiu sp, sp, 16
+ RunCode(&codegenMIPS, graph, [](HGraph*) {}, false, 0);
+}
+#endif
+
} // namespace art