[InstSimplify][ConstantFolding] Teach constant folding how to handle icmp null, (inttoptr x) as well as it handles icmp (inttoptr x), null

[craig.topper]

Summary

The constant folding code currently assumes that the constant expression will always be on the left and the simple null will be on the right. But that's not true at least on the path from InstSimplify.

This patch adds support to ConstantFolding to detect the reversed case.

[davide]

This is exactly the same transform, just with the operands commuted, isn't it? Is there a way to avoid the code duplication & share the two codepaths?
That would be great if we find a bug in one of them, so that we don't have to replicate the fix in the other.

Thanks!

[craig.topper]

Implement with no code duplication. Just detect that LHS is not a constant expr, but RHS is. If that's the case swap the predicate and the operands and recurse back into the same function.

[joey]

LGTM!

llvm/trunk/lib/Analysis/ConstantFolding.cpp

Constant *llvm::ConstantFoldInstOperands(Instruction *I,
   return ConstantFoldInstOperandsImpl(I, I->getOpcode(), Ops, DL, TLI);
 }
 
 Constant *llvm::ConstantFoldCompareInstOperands(unsigned Predicate,
                                                 Constant *Ops0, Constant *Ops1,
                                                 const DataLayout &DL,
                                                 const TargetLibraryInfo *TLI) {
   // fold: icmp (inttoptr x), null         -> icmp x, 0
+  // fold: icmp null, (inttoptr x)         -> icmp 0, x
   // fold: icmp (ptrtoint x), 0            -> icmp x, null
+  // fold: icmp 0, (ptrtoint x)            -> icmp null, x
   // fold: icmp (inttoptr x), (inttoptr y) -> icmp trunc/zext x, trunc/zext y
   // fold: icmp (ptrtoint x), (ptrtoint y) -> icmp x, y
   //
   // FIXME: The following comment is out of data and the DataLayout is here now.
   // ConstantExpr::getCompare cannot do this, because it doesn't have DL
   // around to know if bit truncation is happening.
   if (auto *CE0 = dyn_cast<ConstantExpr>(Ops0)) {
     if (Ops1->isNullValue()) {
if ((Predicate == ICmpInst::ICMP_EQ || Predicate == ICmpInst::ICMP_NE) &&
       Constant *LHS = ConstantFoldCompareInstOperands(
           Predicate, CE0->getOperand(0), Ops1, DL, TLI);
       Constant *RHS = ConstantFoldCompareInstOperands(
           Predicate, CE0->getOperand(1), Ops1, DL, TLI);
       unsigned OpC =
         Predicate == ICmpInst::ICMP_EQ ? Instruction::And : Instruction::Or;
       return ConstantFoldBinaryOpOperands(OpC, LHS, RHS, DL);
     }
+  } else if (isa<ConstantExpr>(Ops1)) {
+    // If RHS is a constant expression, but the left side isn't, swap the
+    // operands and try again.
+    Predicate = ICmpInst::getSwappedPredicate((ICmpInst::Predicate)Predicate);
+    return ConstantFoldCompareInstOperands(Predicate, Ops1, Ops0, DL, TLI);
   }
 
   return ConstantExpr::getCompare(Predicate, Ops0, Ops1);
 }
 
 Constant *llvm::ConstantFoldBinaryOpOperands(unsigned Opcode, Constant *LHS,
                                              Constant *RHS,
                                              const DataLayout &DL) {

llvm/trunk/test/Transforms/InstSimplify/compare.ll

 ; CHECK-NEXT:    ret i1 false
 ;
   %x = icmp eq i32* inttoptr (i64 32 to i32*), null
   ret i1 %x
 }
 
 define i1 @constant_fold_null_inttoptr() {
 ; CHECK-LABEL: @constant_fold_null_inttoptr(
-; CHECK-NEXT:    ret i1 icmp eq (i32* inttoptr (i64 32 to i32*), i32* null)
+; CHECK-NEXT:    ret i1 false
 ;
   %x = icmp eq i32* null, inttoptr (i64 32 to i32*)
   ret i1 %x
 }