[VectorCombine] Fix trunc generated between PHINodes

[igogo-x86]

No description provided.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Igor Kirillov (igogo-x86)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/108228.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+6-2)
• (modified) llvm/test/Transforms/VectorCombine/AArch64/shrink-types.ll (+27)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 3e7118091c8e5e..e993e5684f4115 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -2652,8 +2652,12 @@ bool VectorCombine::shrinkType(llvm::Instruction &I) {
     return false;
 
   Value *Op0 = ZExted;
-  if (auto *OI = dyn_cast<Instruction>(OtherOperand))
-    Builder.SetInsertPoint(OI->getNextNode());
+  if (auto *OI = dyn_cast<Instruction>(OtherOperand)) {
+    if (isa<PHINode>(OI))
+      Builder.SetInsertPoint(OI->getParent()->getFirstInsertionPt());
+    else
+      Builder.SetInsertPoint(OI->getNextNode());
+  }
   Value *Op1 = Builder.CreateTrunc(OtherOperand, SmallTy);
   Builder.SetInsertPoint(&I);
   // Keep the order of operands the same
diff --git a/llvm/test/Transforms/VectorCombine/AArch64/shrink-types.ll b/llvm/test/Transforms/VectorCombine/AArch64/shrink-types.ll
index 0166656cf734f5..33e295841f641a 100644
--- a/llvm/test/Transforms/VectorCombine/AArch64/shrink-types.ll
+++ b/llvm/test/Transforms/VectorCombine/AArch64/shrink-types.ll
@@ -73,4 +73,31 @@ entry:
   ret i32 %6
 }
 
+define i32 @phi_bug(<16 x i32> %a, ptr %b) {
+; CHECK-LABEL: @phi_bug(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[WIDE_LOAD:%.*]] = load <16 x i8>, ptr [[B:%.*]], align 1
+; CHECK-NEXT:    br label [[VECTOR_BODY:%.*]]
+; CHECK:       vector.body:
+; CHECK-NEXT:    [[A_PHI:%.*]] = phi <16 x i32> [ [[A:%.*]], [[ENTRY:%.*]] ]
+; CHECK-NEXT:    [[WIDE_LOAD_PHI:%.*]] = phi <16 x i8> [ [[WIDE_LOAD]], [[ENTRY]] ]
+; CHECK-NEXT:    [[TMP0:%.*]] = trunc <16 x i32> [[A_PHI]] to <16 x i8>
+; CHECK-NEXT:    [[TMP1:%.*]] = and <16 x i8> [[WIDE_LOAD_PHI]], [[TMP0]]
+; CHECK-NEXT:    [[TMP2:%.*]] = zext <16 x i8> [[TMP1]] to <16 x i32>
+; CHECK-NEXT:    [[TMP3:%.*]] = tail call i32 @llvm.vector.reduce.add.v16i32(<16 x i32> [[TMP2]])
+; CHECK-NEXT:    ret i32 [[TMP3]]
+;
+entry:
+  %wide.load = load <16 x i8>, ptr %b, align 1
+  br label %vector.body
+
+vector.body:
+  %a.phi = phi <16 x i32> [ %a, %entry ]
+  %wide.load.phi = phi <16 x i8> [ %wide.load, %entry ]
+  %0 = zext <16 x i8> %wide.load.phi to <16 x i32>
+  %1 = and <16 x i32> %0, %a.phi
+  %2 = tail call i32 @llvm.vector.reduce.add.v16i32(<16 x i32> %1)
+  ret i32 %2
+}
+
 declare i32 @llvm.vector.reduce.add.v16i32(<16 x i32>)

[mikaelholmen]

Thanks! I've verified that this solves the problem I saw.

[nikic]

You need to use getInsertionPointAfterDef() to handle all the edge cases correctly (I'd expect you will handle invoke and catchswitch incorrectly right now). Or even better yet, just don't do this and create all the instructions at &I. From a cursory look I don't see a need to create this instruction in a different place.

[igogo-x86]

You are right. I think I was worried that trunc might get swept inside the loop but LICM follows not so long after VectorCombine

#108398