[PATCH] D70093: [InstCombine] Avoid moving ops that do restrict undef across shuffles.

[Florian Hahn via Phabricator via llvm-commits]

fhahn created this revision.
fhahn added reviewers: spatel, RKSimon, lebedev.ri.
Herald added a subscriber: hiraditya.
Herald added a project: LLVM.

I think we have to be a bit more careful when it comes to moving
ops across shuffles, if the op does restrict undef. For example, without
this patch, we would move 'and %v, <0, 0, -1, -1>' over a
'shufflevector %a, undef, <undef, undef, 1, 2>'. As a result, the first
2 lanes of the result are undef after the combine, but they really
should be 0, unless I am missing something.

For ops that do fold to undef on undef operands, the current behavior
should be fine. I've add conservative check OpDoesRestrictUndef, maybe
there's a better existing utility?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D70093

Files:
  llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
  llvm/test/Transforms/InstCombine/vec_shuffle.ll


Index: llvm/test/Transforms/InstCombine/vec_shuffle.ll
===================================================================
--- llvm/test/Transforms/InstCombine/vec_shuffle.ll
+++ llvm/test/Transforms/InstCombine/vec_shuffle.ll
@@ -1004,10 +1004,13 @@
   ret <2 x i32> %r
 }
 
+; AND does not fold to undef for undef operands, we cannot move it
+; across a shuffle with undef masks.
 define <4 x i16> @and_constant_mask_undef(<4 x i16> %add) {
 ; CHECK-LABEL: @and_constant_mask_undef(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[AND:%.*]] = shufflevector <4 x i16> [[ADD:%.*]], <4 x i16> undef, <4 x i32> <i32 undef, i32 undef, i32 1, i32 1>
+; CHECK-NEXT:    [[SHUFFLE:%.*]] = shufflevector <4 x i16> [[ADD:%.*]], <4 x i16> undef, <4 x i32> <i32 undef, i32 undef, i32 1, i32 1>
+; CHECK-NEXT:    [[AND:%.*]] = and <4 x i16> [[SHUFFLE]], <i16 0, i16 0, i16 -1, i16 -1>
 ; CHECK-NEXT:    ret <4 x i16> [[AND]]
 ;
 entry:
Index: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1497,6 +1497,21 @@
     }
   }
 
+  // Returns true, if the binary operator restricts undef operands, i.e. may no fold into
+  // undef for undef operands, like 'and undef, %x' != undef.
+  auto OpDoesRestrictUndef = [](BinaryOperator::BinaryOps Opcode) {
+    // Conservatively check opcodes that are guaranteed to fold to undef,
+    // for an undef operand.
+    switch (Opcode) {
+      case Instruction::Add:
+      case Instruction::Sub:
+      case Instruction::Shl:
+        return false;
+    default:
+      return true;
+    }
+  };
+
   // If one argument is a shuffle within one vector and the other is a constant,
   // try moving the shuffle after the binary operation. This canonicalization
   // intends to move shuffles closer to other shuffles and binops closer to
@@ -1539,6 +1554,9 @@
           break;
         }
         NewVecC[ShMask[I]] = CElt;
+      } else if (OpDoesRestrictUndef(Opcode)) {
+        MayChange = false;
+        break;
       }
       // If this is a widening shuffle, we must be able to extend with undef
       // elements. If the original binop does not produce an undef in the high


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D70093.228725.patch
Type: text/x-patch
Size: 2308 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191111/c9e95af0/attachment.bin>