[ConstantFold] Handle identity folds at top of ConstantFoldBinaryInst

[fhahn]

Summary

Currently we miss folds with undef and identity values for binary ops
that do not fold to undef in general.

We can generalize the identity simplifications and do them before
checking for undef in particular.

Alive checks:

• OR - https://rise4fun.com/Alive/8OsK
• AND - https://rise4fun.com/Alive/e3tE

This will also allow us to remove some now redundant cases throughout
the function, but I would like to do this as follow-up. That should make
tracking down potential issues easier.

[spatel]

llvm/lib/IR/ConstantFold.cpp

992

Add a code comment for future reference? An identity constant always allows returning the other operand including undef/poison.
There's some possibly related discussion about that in:
https://bugs.llvm.org/show_bug.cgi?id=43958

Also, we can handle several more identity patterns like:
// X << 0 = X
by passing the optional "AllowRHSConstant" parameter to getBinOpIdentity().

So that could be another TODO for a follow-up patch. Our test coverage seems very lacking for this function though...so that should be improved first to make sure we're not breaking any rules.

llvm/test/Transforms/InstCombine/binop-identity-undef.ll

2 ↗

(On Diff #229051)

This file would be better placed under test/Analysis/ConstantFolding and RUN with -constprop. (Don't need full instcombine or even instsimplify to get these transforms.)

llvm/test/Transforms/InstCombine/vec_shuffle.ll

1037

Here and similarly below - remove and/or adjust the comment line since we handle this now.

[fhahn]

Add additional comment and FIXME to ConstantFold.cpp, move test to Analysis/ConstantFolding/, drop fixmes from vec_shuffle.ll

[spatel]

LGTM

llvm/lib/IR/ConstantFold.cpp

Constant *llvm::ConstantFoldUnaryInstruction(unsigned Opcode, Constant *C) {
   // We don't know how to fold this.
   return nullptr;
 }
 
 Constant *llvm::ConstantFoldBinaryInstruction(unsigned Opcode, Constant *C1,
                                               Constant *C2) {
   assert(Instruction::isBinaryOp(Opcode) && "Non-binary instruction detected");
 
+  // Simplify BinOps with their identity values first. They are no-ops and we
+  // can always return the other value, including undef or poison values.
+  // FIXME: remove unnecessary duplicated identity patterns below.
+  // FIXME: Use AllowRHSConstant with getBinOpIdentity to handle additional ops,
+  //        like X << 0 = X.
+  Constant *Identity = ConstantExpr::getBinOpIdentity(Opcode, C1->getType());
+  if (Identity) {
+    if (C1 == Identity)
+      return C2;
+    if (C2 == Identity)
+      return C1;
+  }
+
   // Handle scalar UndefValue. Vectors are always evaluated per element.
   bool HasScalarUndef = !C1->getType()->isVectorTy() &&
                         (isa<UndefValue>(C1) || isa<UndefValue>(C2));
   if (HasScalarUndef) {
     switch (static_cast<Instruction::BinaryOps>(Opcode)) {
     case Instruction::Xor:
       if (isa<UndefValue>(C1) && isa<UndefValue>(C2))
         // Handle undef ^ undef -> 0 special case. This is a common

llvm/test/Analysis/ConstantFolding/binop-identity-undef.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -constprop -S %s | FileCheck %s
 
 define i32 @and1() {
 ; CHECK-LABEL: @and1(
-; CHECK-NEXT:    ret i32 0
+; CHECK-NEXT:    ret i32 undef
 ;
   %r = and i32 undef, -1
   ret i32 %r
 }
 
 define i32 @and2() {
 ; CHECK-LABEL: @and2(
-; CHECK-NEXT:    ret i32 0
+; CHECK-NEXT:    ret i32 undef
 ;
   %r = and i32 -1, undef
   ret i32 %r
 }
 
 define i32 @and3_no_identity() {
 ; CHECK-LABEL: @and3_no_identity(
 ; CHECK-NEXT:    ret i32 0
 ;
   %r = and i32 10, undef
   ret i32 %r
 }
 
 define i32 @or1() {
 ; CHECK-LABEL: @or1(
-; CHECK-NEXT:    ret i32 -1
+; CHECK-NEXT:    ret i32 undef
 ;
   %r = or i32 0, undef
   ret i32 %r
 }
 
 define i32 @or2() {
 ; CHECK-LABEL: @or2(
-; CHECK-NEXT:    ret i32 -1
+; CHECK-NEXT:    ret i32 undef
 ;
   %r = or i32 undef, 0
   ret i32 %r
 }
 
 define i32 @or3_no_identity() {
 ; CHECK-LABEL: @or3_no_identity(
 ; CHECK-NEXT:    ret i32 -1
 ;
   %r = or i32 undef, 10
   ret i32 %r
 }

llvm/test/Transforms/InstCombine/vec_shuffle.ll

 ; CHECK-NEXT:    ret <4 x i16> [[AND]]
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %add, <4 x i16> undef, <4 x i32> <i32 1, i32 1, i32 1, i32 undef>
   %and = and <4 x i16> %shuffle, <i16 -1, i16 -1, i16 -1, i16 -0>
   ret <4 x i16> %and
 }
 
-; FIXME: We should be able to move the AND across the shuffle, as -1 (AND identity value) is used for undef lanes.
+; We can move the AND across the shuffle, as -1 (AND identity value) is used for undef lanes.
 define <4 x i16> @and_constant_mask_undef_3(<4 x i16> %add) {
 ; CHECK-LABEL: @and_constant_mask_undef_3(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i16> [[ADD:%.*]], <4 x i16> undef, <4 x i32> <i32 0, i32 1, i32 1, i32 undef>
-; CHECK-NEXT:    [[AND:%.*]] = and <4 x i16> [[SHUFFLE]], <i16 0, i16 0, i16 0, i16 -1>
-; CHECK-NEXT:    ret <4 x i16> [[AND]]
+; CHECK-NEXT:    ret <4 x i16> <i16 0, i16 0, i16 0, i16 undef>
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %add, <4 x i16> undef, <4 x i32> <i32 0, i32 1, i32 1, i32 undef>
   %and = and <4 x i16> %shuffle, <i16 0, i16 0, i16 0, i16 -1>
   ret <4 x i16> %and
 }
 
-; FIXME: We should be able to move the AND across the shuffle, as -1 (AND identity value) is used for undef lanes.
+; We can move the AND across the shuffle, as -1 (AND identity value) is used for undef lanes.
 define <4 x i16> @and_constant_mask_undef_4(<4 x i16> %add) {
 ; CHECK-LABEL: @and_constant_mask_undef_4(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i16> [[ADD:%.*]], <4 x i16> undef, <4 x i32> <i32 0, i32 1, i32 1, i32 undef>
-; CHECK-NEXT:    [[AND:%.*]] = and <4 x i16> [[SHUFFLE]], <i16 9, i16 20, i16 20, i16 -1>
+; CHECK-NEXT:    [[TMP0:%.*]] = and <4 x i16> [[ADD:%.*]], <i16 9, i16 20, i16 undef, i16 undef>
+; CHECK-NEXT:    [[AND:%.*]] = shufflevector <4 x i16> [[TMP0]], <4 x i16> undef, <4 x i32> <i32 0, i32 1, i32 1, i32 undef>
 ; CHECK-NEXT:    ret <4 x i16> [[AND]]
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %add, <4 x i16> undef, <4 x i32> <i32 0, i32 1, i32 1, i32 undef>
   %and = and <4 x i16> %shuffle, <i16 9, i16 20, i16 20, i16 -1>
   ret <4 x i16> %and
 }
 

 ; CHECK-NEXT:    ret <4 x i16> [[OR]]
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %in, <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
   %or = or <4 x i16> %shuffle, <i16 -1, i16 0, i16 0, i16 -1>
   ret <4 x i16> %or
 }
 
-; FIXME: We should be able to move the OR across the shuffle, as 0 (OR identity value) is used for undef lanes.
+; We can move the OR across the shuffle, as 0 (OR identity value) is used for undef lanes.
 define <4 x i16> @or_constant_mask_undef_3(<4 x i16> %in) {
 ; CHECK-LABEL: @or_constant_mask_undef_3(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i16> [[IN:%.*]], <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
-; CHECK-NEXT:    [[OR:%.*]] = or <4 x i16> [[SHUFFLE]], <i16 0, i16 -1, i16 -1, i16 0>
-; CHECK-NEXT:    ret <4 x i16> [[OR]]
+; CHECK-NEXT:    ret <4 x i16> <i16 undef, i16 -1, i16 -1, i16 undef>
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %in, <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
   %or = or <4 x i16> %shuffle, <i16 0, i16 -1, i16 -1, i16 0>
   ret <4 x i16> %or
 }
 
-; FIXME: We should be able to move the OR across the shuffle, as 0 (OR identity value) is used for undef lanes.
+; We can move the OR across the shuffle, as 0 (OR identity value) is used for undef lanes.
 define <4 x i16> @or_constant_mask_undef_4(<4 x i16> %in) {
 ; CHECK-LABEL: @or_constant_mask_undef_4(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i16> [[IN:%.*]], <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
-; CHECK-NEXT:    [[OR:%.*]] = or <4 x i16> [[SHUFFLE]], <i16 0, i16 99, i16 99, i16 0>
+; CHECK-NEXT:    [[TMP0:%.*]] = or <4 x i16> [[IN:%.*]], <i16 undef, i16 99, i16 undef, i16 undef>
+; CHECK-NEXT:    [[OR:%.*]] = shufflevector <4 x i16> [[TMP0]], <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
 ; CHECK-NEXT:    ret <4 x i16> [[OR]]
 ;
 entry:
   %shuffle = shufflevector <4 x i16> %in, <4 x i16> undef, <4 x i32> <i32 undef, i32 1, i32 1, i32 undef>
   %or = or <4 x i16> %shuffle, <i16 0, i16 99, i16 99, i16 0>
   ret <4 x i16> %or
 }