[llvm] [GlobalISel] Fold G_ICMP if possible (PR #86357)

Amara Emerson via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 25 14:29:56 PDT 2024


================
@@ -7638,7 +7644,7 @@ LegalizerHelper::lowerSADDO_SSUBO(MachineInstr &MI) {
   // (LHS) if and only if the other operand (RHS) is (non-zero) positive,
   // otherwise there will be overflow.
   auto ResultLowerThanLHS =
-      MIRBuilder.buildICmp(CmpInst::ICMP_SLT, BoolTy, Dst0, LHS);
+      MIRBuilder.buildICmp(CmpInst::ICMP_SLT, BoolTy, Tmp, LHS);
----------------
aemerson wrote:

I didn't realize this issue was quite that prevalent. We don't have an explicit contract with what state the MIR is expected to be in when we use MIRBuilder, and maybe we should.

`Builder.buildInstr(MI.getOperand(0).getReg(), ...);` only works if `buildInstr` is a dumb constructor, but for CSEMIRBuilder that's not true. I'm not totally sure what the right thing to do is. @arsenm any opinions?

Perhaps we should be strict and say that you can't construct another def of a vreg, so `MI.eraseFromParent()` needs to be called before you do so.

Alternatively, maybe CSEMachineIRBuilder can automatically erase the original instruction if any of the `DstOp` operands are `Register`. These should be separate changes from this patch though.

https://github.com/llvm/llvm-project/pull/86357


More information about the llvm-commits mailing list