Skip to content

Commit 6107150

Browse files
alexmalyshevmeta-codesync[bot]
authored andcommitted
Refactor LIR condition codes as a Condition on the instruction
Summary: The 10 comparison opcodes and the 18 conditional branch opcodes each encoded two independent things in their names, a relation and its signedness, plus a separate set of conditions that read a single flag rather than any comparison. Every helper in `ops.cpp` existed to pick those apart again, codegen mapped each opcode to a machine encoding one case at a time, and the bulk property switches listed all 28 in identical runs. Introduce a `Condition` subopcode and describe the whole space in `FOREACH_LIR_CONDITION`, whose columns are the condition, its negation, the condition testing the same relation with the compared operands swapped, and the name it prints under. The 28 opcodes become `Compare` and `BranchCC`, carrying the condition as a field on `Instruction`. What is left of the old helper surface is `negate` and `swapOperands`. A pass that wants the opposite branch writes `negate(instr->condition())`, and one fusing a compare into a branch copies the condition across, so the six conversion functions the opcodes needed are all gone. Per-condition `case` labels go from 233 to 13, and those 13 only name the two opcodes. The condition names are asmjit's `CondCode` aliases, which are spelled the same for x86-64 and aarch64, so codegen needs no per-target table: one generated switch turns a `Condition` into either enum, and the four hand-written encoding switches collapse into `as->set`, `as->cset`, `as->j` and `as->b`. Conditional branches all lower identically, so they are handled once ahead of the opcode switch rather than as 18 cases per target. A `Condition` can be passed alongside the operands, so construction reads `allocateInstr(Opcode::kBranchCC, origin, Condition::kNotZero, Lbl{bb})`. It sets the field instead of adding an input, keeping it out of the register allocator's way. `A64GuardCC` stops round-tripping an `Opcode` through an `Imm` operand as a result. LIR still prints and parses under the old per-condition names: `opname()` consults the condition, and the parser maps each name to an opcode and a condition. Dumps are unchanged. Switching to a `BranchCC<SignedGE>` spelling is a one-line change in the printer if that reads better. Also deletes `flipBranchCCDirection`, which had no callers. Reviewed By: DinoV Differential Revision: D116078209 fbshipit-source-id: 4f047b6c484d6aa2c351099533507157db92e6ca
1 parent a6be575 commit 6107150

20 files changed

Lines changed: 899 additions & 836 deletions

cinderx/Jit/codegen/autogen.cpp

Lines changed: 54 additions & 233 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,39 @@ namespace cinderx::jit::codegen::autogen {
2525

2626
namespace {
2727

28+
#if defined(CINDER_X86_64)
29+
using AsmCondCode = x86::CondCode;
30+
#elif defined(CINDER_AARCH64)
31+
using AsmCondCode = arm::CondCode;
32+
#endif
33+
34+
#if defined(CINDER_X86_64) || defined(CINDER_AARCH64)
35+
// LIR spells its conditions the same way asmjit does, on both targets.
36+
AsmCondCode asmCondCode(lir::Condition cond) {
37+
switch (cond) {
38+
#define TO_ASMJIT(NAME, ...) \
39+
case lir::Condition::k##NAME: \
40+
return AsmCondCode::k##NAME;
41+
FOREACH_LIR_CONDITION(TO_ASMJIT)
42+
#undef TO_ASMJIT
43+
case lir::Condition::kInvalid:
44+
break;
45+
}
46+
JIT_THROW("Cannot encode invalid condition code {}", static_cast<int>(cond));
47+
}
48+
49+
void emitBranchCC(
50+
arch::Builder* as,
51+
lir::Condition cond,
52+
const asmjit::Label& label) {
53+
#if defined(CINDER_X86_64)
54+
as->j(asmCondCode(cond), label);
55+
#else
56+
as->b(asmCondCode(cond), label);
57+
#endif
58+
}
59+
#endif
60+
2861
bool isMemoryMoveOperand(const lir::Operand* operand) {
2962
return operand->isStack() || operand->isMem() || operand->isInd();
3063
}
@@ -271,75 +304,13 @@ void fillCallSiteLiveValueLocations(Environ* env, const Instruction* instr) {
271304
} // namespace
272305

273306
#if defined(CINDER_AARCH64)
274-
void translateBranchCC(
275-
a64::Builder* as,
276-
Opcode opcode,
277-
const asmjit::Label& label) {
278-
switch (opcode) {
279-
case Opcode::kBranchZ:
280-
case Opcode::kBranchE:
281-
as->b_eq(label);
282-
break;
283-
case Opcode::kBranchNZ:
284-
case Opcode::kBranchNE:
285-
as->b_ne(label);
286-
break;
287-
case Opcode::kBranchC:
288-
as->b_cs(label);
289-
break;
290-
case Opcode::kBranchNC:
291-
as->b_cc(label);
292-
break;
293-
case Opcode::kBranchO:
294-
as->b_vs(label);
295-
break;
296-
case Opcode::kBranchNO:
297-
as->b_vc(label);
298-
break;
299-
case Opcode::kBranchS:
300-
as->b_mi(label);
301-
break;
302-
case Opcode::kBranchNS:
303-
as->b_pl(label);
304-
break;
305-
case Opcode::kBranchA:
306-
as->b_hi(label);
307-
break;
308-
case Opcode::kBranchB:
309-
as->b_lo(label);
310-
break;
311-
case Opcode::kBranchAE:
312-
as->b_hs(label);
313-
break;
314-
case Opcode::kBranchBE:
315-
as->b_ls(label);
316-
break;
317-
case Opcode::kBranchG:
318-
as->b_gt(label);
319-
break;
320-
case Opcode::kBranchL:
321-
as->b_lt(label);
322-
break;
323-
case Opcode::kBranchGE:
324-
as->b_ge(label);
325-
break;
326-
case Opcode::kBranchLE:
327-
as->b_le(label);
328-
break;
329-
default:
330-
JIT_ABORT(
331-
"Unsupported AArch64 condition branch opcode {}",
332-
static_cast<int>(opcode));
333-
}
334-
}
335-
336307
void translateA64GuardCC(Environ* env, const Instruction* instr) {
337308
auto index = static_cast<size_t>(instr->getInput(1)->getConstant());
338309
auto* block = map_get(env->deopt_exit_blocks, index);
339310
auto label = map_get(env->block_label_map, block);
340-
auto opcode = static_cast<Opcode>(instr->getInput(0)->getConstant());
311+
auto cond = static_cast<lir::Condition>(instr->getInput(0)->getConstant());
341312

342-
translateBranchCC(env->as, opcode, label);
313+
emitBranchCC(env->as, cond, label);
343314
fillLiveValueLocations(env->code_rt, index, instr, 2, instr->getNumInputs());
344315
}
345316
#endif
@@ -566,48 +537,15 @@ void TranslateCompare(Environ* env, const Instruction* instr) {
566537
} else {
567538
// Floating-point comparison; both operands are in XMM registers. `comisd`
568539
// sets the flags in the unsigned sense (CF/ZF) and reports unordered (NaN)
569-
// operands as CF=ZF=PF=1; the shared condition switch below then reads
570-
// those flags. NaN-correctness and the comparison direction are chosen
571-
// when the compare is lowered to LIR, so a compare fused into a branch,
572-
// which reuses these flags via compareToBranchCC on the LIR opcode, stays
573-
// consistent with the standalone setcc emitted here.
540+
// operands as CF=ZF=PF=1; the setcc below then reads those flags.
541+
// NaN-correctness and the comparison direction are chosen when the compare
542+
// is lowered to LIR, so a compare fused into a branch, which reuses these
543+
// flags via compareToBranchCC on the LIR opcode, stays consistent with the
544+
// standalone setcc emitted here.
574545
as->comisd(AutoTranslator::getVecD(inp0), AutoTranslator::getVecD(inp1));
575546
}
576547
auto output = AutoTranslator::getGp(instr->output());
577-
switch (instr->opcode()) {
578-
case Opcode::kEqual:
579-
as->sete(output);
580-
break;
581-
case Opcode::kNotEqual:
582-
as->setne(output);
583-
break;
584-
case Opcode::kGreaterThanSigned:
585-
as->setg(output);
586-
break;
587-
case Opcode::kGreaterThanEqualSigned:
588-
as->setge(output);
589-
break;
590-
case Opcode::kLessThanSigned:
591-
as->setl(output);
592-
break;
593-
case Opcode::kLessThanEqualSigned:
594-
as->setle(output);
595-
break;
596-
case Opcode::kGreaterThanUnsigned:
597-
as->seta(output);
598-
break;
599-
case Opcode::kGreaterThanEqualUnsigned:
600-
as->setae(output);
601-
break;
602-
case Opcode::kLessThanUnsigned:
603-
as->setb(output);
604-
break;
605-
case Opcode::kLessThanEqualUnsigned:
606-
as->setbe(output);
607-
break;
608-
default:
609-
JIT_ABORT("Bad instruction for TranslateCompare {}", instr->opname());
610-
}
548+
as->set(asmCondCode(instr->condition()), output);
611549
if (instr->output()->dataType() != lir::Operand::k8bit) {
612550
as->movzx(
613551
AutoTranslator::getGp(instr->output()),
@@ -634,48 +572,15 @@ void TranslateCompare(Environ* env, const Instruction* instr) {
634572
as->cmp(AutoTranslator::getGpWiden(inp0), AutoTranslator::getGpWiden(inp1));
635573
} else {
636574
// Floating-point comparison, see the note in the x86-64 path. `fcmp` sets
637-
// NZCV (unordered/NaN operands set C=1, V=1 while leaving Z=0), the shared
638-
// condition switch below picks the cset. NaN-correctness and the comparison
575+
// NZCV (unordered/NaN operands set C=1, V=1 while leaving Z=0), and the
576+
// cset below reads them. NaN-correctness and the comparison
639577
// direction are chosen when the compare is lowered to LIR, keeping the
640578
// standalone cset and any fused b.cc consistent.
641579
as->fcmp(AutoTranslator::getVecD(inp0), AutoTranslator::getVecD(inp1));
642580
}
643581

644582
auto output = AutoTranslator::getGpOutput(instr->output());
645-
switch (instr->opcode()) {
646-
case Opcode::kEqual:
647-
as->cset(output, arm::CondCode::kEQ);
648-
break;
649-
case Opcode::kNotEqual:
650-
as->cset(output, arm::CondCode::kNE);
651-
break;
652-
case Opcode::kGreaterThanSigned:
653-
as->cset(output, arm::CondCode::kGT);
654-
break;
655-
case Opcode::kGreaterThanEqualSigned:
656-
as->cset(output, arm::CondCode::kGE);
657-
break;
658-
case Opcode::kLessThanSigned:
659-
as->cset(output, arm::CondCode::kLT);
660-
break;
661-
case Opcode::kLessThanEqualSigned:
662-
as->cset(output, arm::CondCode::kLE);
663-
break;
664-
case Opcode::kGreaterThanUnsigned:
665-
as->cset(output, arm::CondCode::kHI);
666-
break;
667-
case Opcode::kGreaterThanEqualUnsigned:
668-
as->cset(output, arm::CondCode::kHS);
669-
break;
670-
case Opcode::kLessThanUnsigned:
671-
as->cset(output, arm::CondCode::kLO);
672-
break;
673-
case Opcode::kLessThanEqualUnsigned:
674-
as->cset(output, arm::CondCode::kLS);
675-
break;
676-
default:
677-
JIT_ABORT("Bad instruction for TranslateCompare {}", instr->opname());
678-
}
583+
as->cset(output, asmCondCode(instr->condition()));
679584
#else
680585
CINDER_UNSUPPORTED
681586
#endif
@@ -2698,6 +2603,14 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
26982603
}
26992604
#endif
27002605

2606+
// Every conditional branch reads its condition out of the status flags and
2607+
// jumps to the label in its first input, so they all lower the same way.
2608+
if (opcode == Opcode::kBranchCC) {
2609+
emitBranchCC(
2610+
env->as, instr->condition(), getLabel(env, instr->getInput(0)));
2611+
return;
2612+
}
2613+
27012614
switch (opcode) {
27022615
case Opcode::kBind:
27032616
case Opcode::kCallSiteLiveValues:
@@ -2916,60 +2829,6 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
29162829
}
29172830
return;
29182831
}
2919-
case Opcode::kBranchZ:
2920-
env->as->jz(getLabel(env, instr->getInput(0)));
2921-
return;
2922-
case Opcode::kBranchNZ:
2923-
env->as->jnz(getLabel(env, instr->getInput(0)));
2924-
return;
2925-
case Opcode::kBranchA:
2926-
env->as->ja(getLabel(env, instr->getInput(0)));
2927-
return;
2928-
case Opcode::kBranchB:
2929-
env->as->jb(getLabel(env, instr->getInput(0)));
2930-
return;
2931-
case Opcode::kBranchAE:
2932-
env->as->jae(getLabel(env, instr->getInput(0)));
2933-
return;
2934-
case Opcode::kBranchBE:
2935-
env->as->jbe(getLabel(env, instr->getInput(0)));
2936-
return;
2937-
case Opcode::kBranchG:
2938-
env->as->jg(getLabel(env, instr->getInput(0)));
2939-
return;
2940-
case Opcode::kBranchL:
2941-
env->as->jl(getLabel(env, instr->getInput(0)));
2942-
return;
2943-
case Opcode::kBranchGE:
2944-
env->as->jge(getLabel(env, instr->getInput(0)));
2945-
return;
2946-
case Opcode::kBranchLE:
2947-
env->as->jle(getLabel(env, instr->getInput(0)));
2948-
return;
2949-
case Opcode::kBranchC:
2950-
env->as->jc(getLabel(env, instr->getInput(0)));
2951-
return;
2952-
case Opcode::kBranchNC:
2953-
env->as->jnc(getLabel(env, instr->getInput(0)));
2954-
return;
2955-
case Opcode::kBranchO:
2956-
env->as->jo(getLabel(env, instr->getInput(0)));
2957-
return;
2958-
case Opcode::kBranchNO:
2959-
env->as->jno(getLabel(env, instr->getInput(0)));
2960-
return;
2961-
case Opcode::kBranchS:
2962-
env->as->js(getLabel(env, instr->getInput(0)));
2963-
return;
2964-
case Opcode::kBranchNS:
2965-
env->as->jns(getLabel(env, instr->getInput(0)));
2966-
return;
2967-
case Opcode::kBranchE:
2968-
env->as->je(getLabel(env, instr->getInput(0)));
2969-
return;
2970-
case Opcode::kBranchNE:
2971-
env->as->jne(getLabel(env, instr->getInput(0)));
2972-
return;
29732832
case Opcode::kGuard:
29742833
translateGuard(env, instr);
29752834
return;
@@ -3050,16 +2909,7 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
30502909
env->as->cmovnz(output, getReg(instr, instr->getInput(1)));
30512910
return;
30522911
}
3053-
case Opcode::kEqual:
3054-
case Opcode::kNotEqual:
3055-
case Opcode::kGreaterThanUnsigned:
3056-
case Opcode::kGreaterThanEqualUnsigned:
3057-
case Opcode::kLessThanUnsigned:
3058-
case Opcode::kLessThanEqualUnsigned:
3059-
case Opcode::kGreaterThanSigned:
3060-
case Opcode::kGreaterThanEqualSigned:
3061-
case Opcode::kLessThanSigned:
3062-
case Opcode::kLessThanEqualSigned:
2912+
case Opcode::kCompare:
30632913
TranslateCompare(env, instr);
30642914
return;
30652915
case Opcode::kFadd: {
@@ -3397,26 +3247,6 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
33973247
}
33983248
return;
33993249
}
3400-
case Opcode::kBranchC:
3401-
case Opcode::kBranchNC:
3402-
case Opcode::kBranchO:
3403-
case Opcode::kBranchNO:
3404-
case Opcode::kBranchS:
3405-
case Opcode::kBranchNS:
3406-
case Opcode::kBranchZ:
3407-
case Opcode::kBranchNZ:
3408-
case Opcode::kBranchA:
3409-
case Opcode::kBranchB:
3410-
case Opcode::kBranchAE:
3411-
case Opcode::kBranchBE:
3412-
case Opcode::kBranchG:
3413-
case Opcode::kBranchL:
3414-
case Opcode::kBranchGE:
3415-
case Opcode::kBranchLE:
3416-
case Opcode::kBranchE:
3417-
case Opcode::kBranchNE:
3418-
translateBranchCC(env->as, opcode, getLabel(env, instr->getInput(0)));
3419-
return;
34203250
case Opcode::kCmpBranchZero:
34213251
env->as->cbz(
34223252
getGpWiden(instr->getInput(0)), getLabel(env, instr->getInput(1)));
@@ -3475,16 +3305,7 @@ void AutoTranslator::translateInstr(Environ* env, const Instruction* instr)
34753305
case Opcode::kSelect:
34763306
translateSelect(env, instr);
34773307
return;
3478-
case Opcode::kEqual:
3479-
case Opcode::kNotEqual:
3480-
case Opcode::kGreaterThanUnsigned:
3481-
case Opcode::kGreaterThanEqualUnsigned:
3482-
case Opcode::kLessThanUnsigned:
3483-
case Opcode::kLessThanEqualUnsigned:
3484-
case Opcode::kGreaterThanSigned:
3485-
case Opcode::kGreaterThanEqualSigned:
3486-
case Opcode::kLessThanSigned:
3487-
case Opcode::kLessThanEqualSigned:
3308+
case Opcode::kCompare:
34883309
TranslateCompare(env, instr);
34893310
return;
34903311
case Opcode::kFadd: {

cinderx/Jit/lir/block_builder.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,14 @@ class BasicBlockBuilder {
140140
return instr;
141141
}
142142

143+
// Allocate and append a branch on a condition read from the flags.
144+
template <class... Args>
145+
Instruction*
146+
appendBranch(Condition cond, BasicBlock* true_bb, Args&&... args) {
147+
return appendBranch(
148+
Opcode::kBranchCC, true_bb, cond, std::forward<Args>(args)...);
149+
}
150+
143151
template <
144152
typename FuncReturnType,
145153
typename... FuncArgs,

0 commit comments

Comments
 (0)