[InstSimplify] SimplifyICmpInst - icmp eq/ne %X, undef -> undef

[RKSimon]

Summary

As discussed on PR41125 and D59363, we have a mismatch between icmp eq/ne cases with an undef operand:

When the other operand is constant we fold to undef (handled in ConstantFoldCompareInstruction)
When the other operand is non-constant we fold to a bool constant based on isTrueWhenEqual (handled in SimplifyICmpInst).

Neither it really wrong, but this patch changes the logic in SimplifyICmpInst to consistently fold to undef.

The NewGVN test change is annoying (as with most heavily reduced tests) but AFAICT I have kept the purpose of the test based on rL291968.

[spatel]

LGTM

lib/Analysis/InstructionSimplify.cpp

3053 ↗

(On Diff #191271)

If both operands are undef, we should not get here - that should always get constant folded above this.
So I'd remove that line of the code comment. You could add an assert to be extra safe...although that general sequence (handle constant folding first) happens in all or almost all functions within InstSimplify.

llvm/trunk/lib/Analysis/InstructionSimplify.cpp

static Value *SimplifyICmpInst(unsigned Predicate, Value *LHS, Value *RHS,
   if (Constant *CLHS = dyn_cast<Constant>(LHS)) {
     if (Constant *CRHS = dyn_cast<Constant>(RHS))
       return ConstantFoldCompareInstOperands(Pred, CLHS, CRHS, Q.DL, Q.TLI);
 
     // If we have a constant, make sure it is on the RHS.
     std::swap(LHS, RHS);
     Pred = CmpInst::getSwappedPredicate(Pred);
   }
+  assert(!isa<UndefValue>(LHS) && "Unexpected icmp undef,%X");
 
   Type *ITy = GetCompareTy(LHS); // The return type.
 
+  // For EQ and NE, we can always pick a value for the undef to make the
+  // predicate pass or fail, so we can return undef.
+  // Matches behavior in llvm::ConstantFoldCompareInstruction.
+  if (isa<UndefValue>(RHS) && ICmpInst::isEquality(Pred))
+    return UndefValue::get(ITy);
+
   // icmp X, X -> true/false
   // icmp X, undef -> true/false because undef could be X.
   if (LHS == RHS || isa<UndefValue>(RHS))
     return ConstantInt::get(ITy, CmpInst::isTrueWhenEqual(Pred));
 
   if (Value *V = simplifyICmpOfBools(Pred, LHS, RHS, Q))
     return V;
 

llvm/trunk/test/Transforms/InstCombine/icmp.ll

;
   %a = icmp ult i32 %X, -2147483648
   %b = sext i1 %a to i32
   ret i32 %b
 }
 
 ; PR4837
 define <2 x i1> @test5_eq(<2 x i64> %x) {
 ; CHECK-LABEL: @test5_eq(
-; CHECK-NEXT:    ret <2 x i1> <i1 true, i1 true>
+; CHECK-NEXT:    ret <2 x i1> undef
 ;
   %V = icmp eq <2 x i64> %x, undef
   ret <2 x i1> %V
 }
 define <2 x i1> @test5_ne(<2 x i64> %x) {
 ; CHECK-LABEL: @test5_ne(
-; CHECK-NEXT:    ret <2 x i1> zeroinitializer
+; CHECK-NEXT:    ret <2 x i1> undef
 ;
   %V = icmp ne <2 x i64> %x, undef
   ret <2 x i1> %V
 }
 define <2 x i1> @test5_ugt(<2 x i64> %x) {
 ; CHECK-LABEL: @test5_ugt(
 ; CHECK-NEXT:    ret <2 x i1> zeroinitializer
 ;

llvm/trunk/test/Transforms/NewGVN/pr31613.ll

bb18:                                             ; preds = %bb4
   br label %bb1
 }
 
 %struct.a = type {}
 %struct.b = type {}
 
 declare void @c.d.p(i64, i8*)
 
-define void @e() {
+define void @e(i32 %a0, i32 %a1, %struct.a** %p2) {
 ; CHECK-LABEL: @e(
 ; CHECK-NEXT:    [[F:%.*]] = alloca i32
-; CHECK-NEXT:    store i32 undef, i32* [[F]], !g !0
+; CHECK-NEXT:    store i32 [[A0:%.*]], i32* [[F]], !g !0
 ; CHECK-NEXT:    br label [[H:%.*]]
 ; CHECK:       h:
 ; CHECK-NEXT:    call void @c.d.p(i64 8, i8* undef)
+; CHECK-NEXT:    [[I:%.*]] = load i32, i32* [[F]]
 ; CHECK-NEXT:    [[J:%.*]] = load i32, i32* null
-; CHECK-NEXT:    br i1 true, label [[L:%.*]], label [[Q:%.*]]
+; CHECK-NEXT:    [[K:%.*]] = icmp eq i32 [[I]], [[J]]
+; CHECK-NEXT:    br i1 [[K]], label [[L:%.*]], label [[Q:%.*]]
 ; CHECK:       l:
 ; CHECK-NEXT:    br label [[R:%.*]]
 ; CHECK:       q:
-; CHECK-NEXT:    store i8 undef, i8* null
+; CHECK-NEXT:    [[M:%.*]] = load %struct.a*, %struct.a** null
 ; CHECK-NEXT:    br label [[R]]
 ; CHECK:       r:
 ; CHECK-NEXT:    switch i32 undef, label [[N:%.*]] [
 ; CHECK-NEXT:    i32 0, label [[S:%.*]]
 ; CHECK-NEXT:    ]
 ; CHECK:       s:
+; CHECK-NEXT:    store i32 [[A1:%.*]], i32* [[F]], !g !0
 ; CHECK-NEXT:    br label [[H]]
 ; CHECK:       n:
-; CHECK-NEXT:    [[O:%.*]] = load %struct.a*, %struct.a** null
+; CHECK-NEXT:    [[O:%.*]] = load %struct.a*, %struct.a** [[P2:%.*]]
 ; CHECK-NEXT:    ret void
 ;
   %f = alloca i32
-  store i32 undef, i32* %f, !g !0
+  store i32 %a0, i32* %f, !g !0
   br label %h
 
 h:                                                ; preds = %s, %0
   call void @c.d.p(i64 8, i8* undef)
   %i = load i32, i32* %f
   %j = load i32, i32* null
   %k = icmp eq i32 %i, %j
   br i1 %k, label %l, label %q
 
 l:                                                ; preds = %h
   br label %r
 
 q:                                                ; preds = %h
   %m = load %struct.a*, %struct.a** null
   %1 = bitcast %struct.a* %m to %struct.b*
   br label %r
 
 r:                                                ; preds = %q, %l
   switch i32 undef, label %n [
   i32 0, label %s
   ]
 
 s:                                                ; preds = %r
-  store i32 undef, i32* %f, !g !0
+  store i32 %a1, i32* %f, !g !0
   br label %h
 
 n:                                                ; preds = %r
-  %o = load %struct.a*, %struct.a** null
+  %o = load %struct.a*, %struct.a** %p2
   %2 = bitcast %struct.a* %o to %struct.b*
   ret void
 }
 
 !0 = !{}