Skip to content

Commit c95c9cd

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Div fixes
Summary: More widening and fix division which depends upon sign. Reviewed By: alexmalyshev Differential Revision: D93906168 fbshipit-source-id: f54461455b5f5c52e25b43064f83204f35e22ff8
1 parent a374ecd commit c95c9cd

2 files changed

Lines changed: 50 additions & 22 deletions

File tree

cinderx/Jit/codegen/autogen.cpp

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2140,6 +2140,12 @@ void translateMove(Environ* env, const Instruction* instr) {
21402140
as->mov(a64::w(scratch1.id()), input->getConstant());
21412141
as->strh(a64::w(scratch1.id()), ptr);
21422142
break;
2143+
case OperandBase::k32bit:
2144+
// Use w register for 4-byte store to avoid overflowing
2145+
// tightly-packed fields.
2146+
as->mov(a64::w(scratch1.id()), input->getConstant());
2147+
as->str(a64::w(scratch1.id()), ptr);
2148+
break;
21432149
default:
21442150
as->mov(scratch1, input->getConstant());
21452151
as->str(scratch1, ptr);
@@ -2254,7 +2260,7 @@ void translateMovSXD(Environ* env, const Instruction* instr) {
22542260
const OperandBase* input = instr->getInput(0);
22552261

22562262
if (input->isReg()) {
2257-
auto input_reg = AT::getGp(input);
2263+
auto input_reg = asmjit::a64::w(input->getPhyRegister().loc);
22582264
as->sxtw(output, input_reg);
22592265
} else if (input->isStack()) {
22602266
auto loc = input->getStackSlot().loc;
@@ -2418,20 +2424,32 @@ void translateDivOp(
24182424

24192425
const OperandBase* output =
24202426
instr->getNumOutputs() > 0 ? instr->output() : instr->getInput(0);
2421-
const OperandBase* opnd0 = instr->getInput(0);
2422-
const OperandBase* opnd1 = instr->getInput(1);
2427+
2428+
// Division instructions may have an extra leading Imm{0} input (used by x86
2429+
// for the high half of the dividend). Skip it on AArch64.
2430+
size_t base = 0;
2431+
if (instr->getNumInputs() == 3 && instr->getInput(0)->isImm()) {
2432+
base = 1;
2433+
}
2434+
const OperandBase* opnd0 = instr->getInput(base);
2435+
const OperandBase* opnd1 = instr->getInput(base + 1);
24232436

24242437
JIT_CHECK(output->isReg(), "Expected output to be a register");
24252438
JIT_CHECK(opnd0->isReg(), "Expected opnd0 to be a register");
24262439

2427-
auto output_reg = AT::getGpWiden(output);
2428-
auto opnd0_reg = AT::getGpWiden(opnd0);
2440+
// Use getGpOutput to get the correct register width. sdiv/udiv require all
2441+
// operands to be the same width. getGpOutput returns w(reg) for k32bit and
2442+
// x(reg) for k64bit, matching the hardware instruction requirements.
2443+
// (getGpWiden would return x(reg) for k32bit, causing sdiv to interpret
2444+
// zero-extended 32-bit values as 64-bit, giving wrong results for negatives.)
2445+
auto output_reg = AT::getGpOutput(output);
2446+
auto opnd0_reg = AT::getGpOutput(opnd0);
24292447

24302448
if (opnd1->isReg()) {
2431-
emit(as, output_reg, opnd0_reg, AT::getGpWiden(opnd1));
2449+
emit(as, output_reg, opnd0_reg, AT::getGpOutput(opnd1));
24322450
} else if (opnd1->isStack()) {
24332451
auto loc = opnd1->getStackSlot().loc;
2434-
auto scratch = AT::getGpWiden(output->dataType(), arch::reg_scratch_0.id());
2452+
auto scratch = AT::getGpOutput(output, arch::reg_scratch_0.id());
24352453
auto ptr = arch::ptr_resolve(as, arch::fp, loc, arch::reg_scratch_0);
24362454
as->ldr(scratch, ptr);
24372455
emit(as, output_reg, opnd0_reg, scratch);
@@ -2790,13 +2808,17 @@ BEGIN_RULES(Instruction::kDiv)
27902808
GEN("rrm", CALL_C(translateDiv))
27912809
GEN("rr", CALL_C(translateDiv))
27922810
GEN("rm", CALL_C(translateDiv))
2811+
GEN("Rirr", CALL_C(translateDiv))
2812+
GEN("Rirm", CALL_C(translateDiv))
27932813
END_RULES
27942814

27952815
BEGIN_RULES(Instruction::kDivUn)
27962816
GEN("rrr", CALL_C(translateDivUn))
27972817
GEN("rrm", CALL_C(translateDivUn))
27982818
GEN("rr", CALL_C(translateDivUn))
27992819
GEN("rm", CALL_C(translateDivUn))
2820+
GEN("Rirr", CALL_C(translateDivUn))
2821+
GEN("Rirm", CALL_C(translateDivUn))
28002822
END_RULES
28012823

28022824
BEGIN_RULES(Instruction::kFadd)

cinderx/Jit/lir/postgen.cpp

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,29 @@ RewriteResult rewriteBinaryOpConstantPosition(instr_iter_t instr_iter) {
2828
auto block = instr->basicblock();
2929

3030
if (instr->isDiv() || instr->isDivUn()) {
31-
auto divisor = instr->getInput(2);
32-
if (!divisor->isImm()) {
33-
return kUnchanged;
34-
}
35-
36-
// div doesn't support an immediate as the divisor.
37-
auto constant = divisor->getConstant();
38-
auto constant_size = divisor->dataType();
31+
bool changed = false;
32+
// div/sdiv/udiv don't support immediate operands on AArch64.
33+
// Input layout is [Imm{0}, dividend, divisor] where input 0 is the x86
34+
// high-half placeholder. Convert both dividend (input 1) and divisor
35+
// (input 2) to registers if they are immediates.
36+
for (int idx = 1; idx <= 2; idx++) {
37+
auto operand = instr->getInput(idx);
38+
if (!operand->isImm()) {
39+
continue;
40+
}
41+
auto constant = operand->getConstant();
42+
auto constant_size = operand->dataType();
3943

40-
auto move = block->allocateInstrBefore(
41-
instr_iter,
42-
Instruction::kMove,
43-
OutVReg{constant_size},
44-
Imm{constant, constant_size});
44+
auto move = block->allocateInstrBefore(
45+
instr_iter,
46+
Instruction::kMove,
47+
OutVReg{constant_size},
48+
Imm{constant, constant_size});
4549

46-
instr->setInput(2, std::make_unique<LinkedOperand>(move));
47-
return kChanged;
50+
instr->setInput(idx, std::make_unique<LinkedOperand>(move));
51+
changed = true;
52+
}
53+
return changed ? kChanged : kUnchanged;
4854
}
4955

5056
if (!instr->isAdd() && !instr->isSub() && !instr->isXor() &&

0 commit comments

Comments
 (0)