Skip to content

Commit 7c8804c

Browse files
kddnewtonmeta-codesync[bot]
authored andcommitted
Use StorePair for AArch64 regular call arg packing
Summary: Extend the AArch64 post-register-allocation call rewrite to use `StorePair` for adjacent 64-bit general-purpose register arguments that overflow onto the regular call stack argument area. This keeps the existing single-store path for FP, immediate, stack, or otherwise unsupported operands. Reviewed By: DinoV Differential Revision: D109037584 fbshipit-source-id: cf8f0e044442d0db0ad40dfb3f1b8537ff9f01e6
1 parent 67dbf1b commit 7c8804c

4 files changed

Lines changed: 94 additions & 1 deletion

File tree

cinderx/Jit/codegen/autogen.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1405,7 +1405,8 @@ void translateStorePair(Environ* env, const Instruction* instr) {
14051405
x86::qword_ptr(base, offset + kPointerSize),
14061406
x86::gpq(instr->getInput(3)->getPhyRegister().loc));
14071407
#elif defined(CINDER_AARCH64)
1408-
auto base = a64::x(instr->getInput(1)->getPhyRegister().loc);
1408+
auto base_reg = instr->getInput(1)->getPhyRegister();
1409+
auto base = base_reg == SP ? a64::sp : a64::x(base_reg.loc);
14091410
// stp signed offset range is -512..504. Fall back to two str instructions
14101411
// when the offset is out of range.
14111412
if (Support::isInt7(offset >> 3)) {

cinderx/Jit/lir/postalloc.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,24 @@ int rewriteRegularFunction(instr_iter_t instr_iter, int base_offset) {
220220
move->output()->setDataType(operand->dataType());
221221
move->appendInput(instr->releaseInput(i));
222222
} else {
223+
#if defined(CINDER_AARCH64)
224+
if (i + 1 < num_inputs) {
225+
auto next_arg = instr->getInput(i + 1);
226+
if (canStorePairOperand(operand) && canStorePairOperand(next_arg)) {
227+
insertStorePairToMemoryLocation(
228+
block,
229+
instr_iter,
230+
arch::reg_stack_pointer_loc,
231+
base_offset + stack_arg_size,
232+
operand,
233+
next_arg);
234+
++i;
235+
stack_arg_size += 2 * sizeof(void*);
236+
continue;
237+
}
238+
}
239+
#endif
240+
223241
insertMoveToMemoryLocation(
224242
block,
225243
instr_iter,

cinderx/RuntimeTests/lir_abi_test.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,42 @@ TEST_F(LIRABITest, TestkCall_PhyReg) {
216216
translateInstr(Instruction::kCall, makePhyReg());
217217
}
218218

219+
#if defined(CINDER_AARCH64)
220+
TEST_F(LIRABITest, TestkStorePair_SPBase) {
221+
hir::Function hir_function;
222+
223+
Environ environ;
224+
environ.ctx = getContext();
225+
environ.code_rt = environ.ctx->allocateCodeRuntime(
226+
hir_function.code.get(),
227+
hir_function.builtins.get(),
228+
hir_function.globals.get());
229+
230+
auto code_allocator = std::unique_ptr<ICodeAllocator>(CodeAllocator::make());
231+
232+
CodeHolder code;
233+
code.init(code_allocator->asmJitEnvironment());
234+
235+
arch::Builder as(&code);
236+
environ.as = &as;
237+
238+
Function function;
239+
BasicBlock bb(&function);
240+
auto* instr = bb.allocateInstr(
241+
Instruction::kStorePair,
242+
nullptr,
243+
Imm{24},
244+
PhyReg{arch::reg_stack_pointer_loc, DataType::k64bit},
245+
PhyReg{X25, DataType::k64bit},
246+
PhyReg{X20, DataType::k64bit});
247+
248+
autogen::AutoTranslator::getInstance().translateInstr(&environ, instr);
249+
250+
EXPECT_EQ(as.finalize(), asmjit::kErrorOk);
251+
EXPECT_EQ(code.textSection()->bufferSize(), 4);
252+
}
253+
#endif
254+
219255
TEST_F(LIRABITest, TestkCall_FillsCallSiteLiveValueLocations) {
220256
if constexpr (!kFreeThreadedBuild) {
221257
GTEST_SKIP() << "Callsite live-value locations are only filled in "

cinderx/RuntimeTests/lir_postalloc_test.cpp

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,44 @@ BB %0
407407
EXPECT_EQ(store_pairs, 2);
408408
ASSERT_TRUE(verifyPostRegAllocInvariants(parsed_func.get(), std::cout));
409409
}
410+
411+
TEST_F(LIRPostAllocRewriteTest, RegularCallArgsUseStorePairForRegisterPairs) {
412+
auto lir_input_str = fmt::format(
413+
R"(Function:
414+
BB %0
415+
Call 123456789, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object, {:>9}:Object
416+
)",
417+
X0,
418+
X1,
419+
X2,
420+
X3,
421+
X4,
422+
X5,
423+
X6,
424+
X7,
425+
X8,
426+
X9);
427+
428+
Parser parser;
429+
auto parsed_func = parser.parse(lir_input_str);
430+
431+
jit::codegen::Environ env;
432+
PostRegAllocRewrite rewrite(parsed_func.get(), &env);
433+
rewrite.run();
434+
435+
const Instruction* store_pair = nullptr;
436+
for (auto& instr : parsed_func->basicblocks().front()->instructions()) {
437+
if (instr->isStorePair()) {
438+
ASSERT_EQ(store_pair, nullptr);
439+
store_pair = instr.get();
440+
}
441+
}
442+
443+
ASSERT_NE(store_pair, nullptr);
444+
EXPECT_EQ(
445+
store_pair->getInput(1)->getPhyRegister(), arch::reg_stack_pointer_loc);
446+
ASSERT_TRUE(verifyPostRegAllocInvariants(parsed_func.get(), std::cout));
447+
}
410448
#endif // CINDER_AARCH64
411449

412450
} // namespace jit::lir

0 commit comments

Comments
 (0)