[PATCH] D86665: [GlobalISel][IRTranslator] Generate better conditional branch lowering.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:437
+  if (!BOp || !(isa<BinaryOperator>(BOp) || isa<CmpInst>(BOp)) ||
+      BOpc != unsigned(Opc) || !BOp->hasOneUse() ||
+      BOp->getParent() != CurBB->getBasicBlock() ||
----------------
static_cast?


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:485
+  } else {
+    assert(Opc == Instruction::And && "Unknown merge op!");
+    // Codegen X & Y as:
----------------
Would it make sense to assert that we have an `Instruction::And` or an `Instruction::Or` at the beginning of the function?

Then we can catch it before any of the code under, say `BOpc != unsigned(Opc)` kicks in and potentially does weird stuff/asserts.


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:522
+IRTranslator::shouldEmitAsBranches(const std::vector<SwitchCG::CaseBlock> &Cases) {
+  if (Cases.size() != 2) return true;
+
----------------
Why?


================
Comment at: llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:565
   }
+  /*
+  // We want a G_BRCOND to the true BB followed by an unconditional branch.
----------------
Remove commented out code?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86665/new/

https://reviews.llvm.org/D86665