Skip to content

Commit d08d4d1

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Add a LoadPair LIR instruction
Summary: Take 2 of D114969843. This gets the same treatment as D116050156 so we use an address adjustment + load pair instead of two ldr's. Adds `kLoadPair`, the load-side counterpart to the existing `kStorePair`, so adjacent 64-bit loads can be emitted as a single `ldp`. It takes the same (offset, base) shape as `kStorePair`, with the first destination as the output and the second as an extra input. On x86 it lowers to two `mov`s. The first user is `PopulateEntryBlock`, which loads the vectorcall arguments from consecutive slots of the argument array on every JIT function entry: ``` mov x10, x1 mov x10, x1 ldr x1, [x10] ldp x1, x2, [x10] ldr x2, [x10, #8] -> ldp x3, x4, [x10, #0x10] ldr x3, [x10, #0x10] ldp x5, x6, [x10, #0x20] ldr x4, [x10, #0x18] ldr x5, [x10, #0x20] ldr x6, [x10, #0x28] ``` One important caveat about this - we have no support for instructions w/ multiple outputs. So this instruction is only available after register allocation where we've previously allocated the output register as part of a load instruction. The current usage runs after register allocation and the next diff adds a post-alloc pass as well. Pairing is limited to aarch64 so x86 output is unchanged. Reviewed By: yoney, alexmalyshev Differential Revision: D116050157 fbshipit-source-id: d6102c84812d46a0fc5495b0b6a7064fcc4a24a8
1 parent fa3a17e commit d08d4d1

4 files changed

Lines changed: 104 additions & 9 deletions

File tree

cinderx/Jit/codegen/autogen.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1552,6 +1552,37 @@ void translateStorePair(Environ* env, const Instruction* instr) {
15521552
#endif
15531553
}
15541554

1555+
void translateLoadPair(Environ* env, const Instruction* instr) {
1556+
arch::Builder* as = env->as;
1557+
JIT_DCHECK(
1558+
instr->getNumInputs() == 3,
1559+
"LoadPair expects exactly 3 inputs (offset, base, dst1)");
1560+
int32_t offset = static_cast<int32_t>(instr->getInput(0)->getConstant());
1561+
auto base_reg = instr->getInput(1)->getPhyRegister();
1562+
auto dst0_loc = instr->output()->getPhyRegister();
1563+
auto dst1_loc = instr->getInput(2)->getPhyRegister();
1564+
1565+
#if defined(CINDER_X86_64)
1566+
auto base = x86::gpq(base_reg.loc);
1567+
as->mov(x86::gpq(dst0_loc.loc), x86::qword_ptr(base, offset));
1568+
as->mov(x86::gpq(dst1_loc.loc), x86::qword_ptr(base, offset + kPointerSize));
1569+
#elif defined(CINDER_AARCH64)
1570+
auto dst0 = a64::x(dst0_loc.loc);
1571+
auto dst1 = a64::x(dst1_loc.loc);
1572+
1573+
if (auto ptr = getPairPtr(env, base_reg, offset)) {
1574+
as->ldp(dst0, dst1, *ptr);
1575+
} else {
1576+
as->ldp(
1577+
dst0,
1578+
dst1,
1579+
getPairScratchPtr(env, base_reg, offset, dst0_loc, dst1_loc));
1580+
}
1581+
#else
1582+
CINDER_UNSUPPORTED
1583+
#endif
1584+
}
1585+
15551586
// Tear down the frame. On x86, this executes 'leave' (mov rsp, rbp; pop rbp).
15561587
// On aarch64, this restores sp from fp and pops the frame record (fp + lr).
15571588
//
@@ -3285,6 +3316,9 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
32853316
case Opcode::kStorePair:
32863317
translateStorePair(env, instr);
32873318
return;
3319+
case Opcode::kLoadPair:
3320+
translateLoadPair(env, instr);
3321+
return;
32883322
case Opcode::kLeave:
32893323
translateLeave(env);
32903324
return;
@@ -3557,6 +3591,9 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
35573591
case Opcode::kStorePair:
35583592
translateStorePair(env, instr);
35593593
return;
3594+
case Opcode::kLoadPair:
3595+
translateLoadPair(env, instr);
3596+
return;
35603597
case Opcode::kLeave:
35613598
translateLeave(env);
35623599
return;

cinderx/Jit/lir/generator.cpp

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -573,28 +573,79 @@ void PopulateEntryBlock(
573573
auto kPointerSize = static_cast<int32_t>(sizeof(void*));
574574

575575
bool has_extra_args = false;
576+
577+
// Consecutive general-purpose arguments live in adjacent slots, so they can
578+
// be loaded a pair at a time. This block is built after the post-allocation
579+
// pairing pass has run, so the pairs are formed here instead. A double
580+
// argument, a gap left by an overflow argument, or a destination that would
581+
// clobber the array pointer ends the run.
582+
#if defined(CINDER_AARCH64)
583+
constexpr bool kCanPairArgLoads = true;
584+
#else
585+
constexpr bool kCanPairArgLoads = false;
586+
#endif
587+
588+
std::optional<std::pair<int32_t, PhyLocation>> pending;
589+
auto flushPending = [&]() {
590+
if (!pending.has_value()) {
591+
return;
592+
}
593+
entry_block->allocateInstr(
594+
Opcode::kMove,
595+
nullptr,
596+
OutPhyReg(pending->second),
597+
Ind(args_reg, pending->first));
598+
pending.reset();
599+
};
600+
576601
for (size_t i = 0; i < arg_locations.size(); i++) {
577602
PhyLocation arg = arg_locations[i];
578603
if (arg == PhyLocation::REG_INVALID) {
579604
has_extra_args = true;
605+
flushPending();
580606
continue;
581607
}
582-
if (arg.isGpRegister()) {
608+
609+
auto offset = static_cast<int32_t>(i * kPointerSize);
610+
611+
if (!arg.isGpRegister()) {
612+
flushPending();
583613
entry_block->allocateInstr(
584614
Opcode::kMove,
585615
nullptr,
586-
OutPhyReg(arg),
587-
Ind(args_reg, static_cast<int32_t>(i * kPointerSize)));
588-
} else {
616+
OutPhyReg(arg, Operand::kDouble),
617+
Ind(args_reg, offset, Operand::kDouble));
618+
continue;
619+
}
620+
621+
// defense in depth: we don't know this will use hardware load pair so
622+
// don't generate load pair if the registers overlap.
623+
if (arg == args_reg) {
624+
flushPending();
589625
entry_block->allocateInstr(
590-
Opcode::kMove,
626+
Opcode::kMove, nullptr, OutPhyReg(arg), Ind(args_reg, offset));
627+
continue;
628+
}
629+
630+
if (kCanPairArgLoads && pending.has_value() && pending->second != arg) {
631+
JIT_DCHECK(
632+
offset == pending->first + kPointerSize,
633+
"pending arg load should be for the preceding slot");
634+
entry_block->allocateInstr(
635+
Opcode::kLoadPair,
591636
nullptr,
592-
OutPhyReg(arg, Operand::kDouble),
593-
Ind(args_reg,
594-
static_cast<int32_t>(i * kPointerSize),
595-
Operand::kDouble));
637+
OutPhyReg(pending->second),
638+
Imm{static_cast<uint64_t>(pending->first)},
639+
PhyReg{args_reg},
640+
PhyReg{arg});
641+
pending.reset();
642+
continue;
596643
}
644+
645+
flushPending();
646+
pending = std::make_pair(offset, arg);
597647
}
648+
flushPending();
598649
if (has_extra_args) {
599650
// Point extra_args register past the register-bound args to the
600651
// start of the overflow args in the vectorcall array.

cinderx/Jit/lir/linear_scan.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,9 @@ void LinearScanAllocator::calculateLiveIntervals() {
469469
continue;
470470
}
471471

472+
JIT_DCHECK(
473+
instr_opcode != Opcode::kLoadPair,
474+
"load pair can only be generated after register allocation");
472475
// output
473476
auto output_opnd = instr->output();
474477
if (output_opnd->isVreg()) {

cinderx/Jit/lir/spill_alloc.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -172,6 +172,10 @@ void SpillAllocator::rewriteInstr(BasicBlock* block, instr_iter_t iter) {
172172
return;
173173
}
174174

175+
JIT_DCHECK(
176+
!instr->isLoadPair(),
177+
"load pair can only be generated after register allocation");
178+
175179
// Generator frame migration: a Move into the frame-pointer register swaps the
176180
// frame pointer between the machine stack and the heap-allocated generator
177181
// data. Handle values that must cross the switch, then fall through so the

0 commit comments

Comments
 (0)