release/23.x: [VectorCombine] Fix foldBitcastOfVPLoad reordering loads (#218336)

[llvmbot]

Backport 2eda652

Requested by: @lukel97

[llvmbot]

@bababuck @wangpc-pp What do you think about merging this PR to the release branch?

[llvmorg-github-actions]

@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: llvmbot

Changes

Backport 2eda652

Requested by: @lukel97

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+1)
• (modified) llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll (+14)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 8648125222e2a..a1dc1b4e954c8 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -6099,6 +6099,7 @@ bool VectorCombine::foldBitcastOfVPLoad(Instruction &I) {
   if (NewCost > OldCost || !NewCost.isValid())
     return false;
 
+  Builder.SetInsertPoint(II);
   unsigned Factor = NewVecCnt.getKnownScalarFactor(OrigVecCnt);
   Value *NewEVL = Builder.CreateNUWMul(EVL, Builder.getInt32(Factor));
   Value *NewMask = Builder.CreateVectorSplat(NewVecCnt, Builder.getTrue());
diff --git a/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll b/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
index 066ce0484b737..f94501d179740 100644
--- a/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
+++ b/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
@@ -84,3 +84,17 @@ define <vscale x 32 x i1> @bitcast_vp_load_i1(ptr %p) {
   %r = bitcast <vscale x 4 x i8> %l to <vscale x 32 x i1>
   ret <vscale x 32 x i1> %r
 }
+
+; Make sure we create the new vp.load in the same place
+define <vscale x 8 x i16> @bitcast_vp_load_insertpoint(ptr %p) {
+; CHECK-LABEL: define <vscale x 8 x i16> @bitcast_vp_load_insertpoint(
+; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT:    [[R:%.*]] = call <vscale x 8 x i16> @llvm.vp.load.nxv8i16.p0(ptr align 16 [[P]], <vscale x 8 x i1> splat (i1 true), i32 4)
+; CHECK-NEXT:    store i32 0, ptr [[P]], align 4
+; CHECK-NEXT:    ret <vscale x 8 x i16> [[R]]
+;
+  %l = call <vscale x 4 x i32> @llvm.vp.load(ptr %p, <vscale x 4 x i1> splat (i1 true), i32 2)
+  store i32 0, ptr %p
+  %r = bitcast <vscale x 4 x i32> %l to <vscale x 8 x i16>
+  ret <vscale x 8 x i16> %r
+}

[llvmorg-github-actions]

@llvm/pr-subscribers-backend-risc-v

Author: llvmbot

Changes

Backport 2eda652

Requested by: @lukel97

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Vectorize/VectorCombine.cpp (+1)
• (modified) llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll (+14)

diff --git a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
index 8648125222e2a..a1dc1b4e954c8 100644
--- a/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
+++ b/llvm/lib/Transforms/Vectorize/VectorCombine.cpp
@@ -6099,6 +6099,7 @@ bool VectorCombine::foldBitcastOfVPLoad(Instruction &I) {
   if (NewCost > OldCost || !NewCost.isValid())
     return false;
 
+  Builder.SetInsertPoint(II);
   unsigned Factor = NewVecCnt.getKnownScalarFactor(OrigVecCnt);
   Value *NewEVL = Builder.CreateNUWMul(EVL, Builder.getInt32(Factor));
   Value *NewMask = Builder.CreateVectorSplat(NewVecCnt, Builder.getTrue());
diff --git a/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll b/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
index 066ce0484b737..f94501d179740 100644
--- a/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
+++ b/llvm/test/Transforms/VectorCombine/RISCV/fold-vp-load.ll
@@ -84,3 +84,17 @@ define <vscale x 32 x i1> @bitcast_vp_load_i1(ptr %p) {
   %r = bitcast <vscale x 4 x i8> %l to <vscale x 32 x i1>
   ret <vscale x 32 x i1> %r
 }
+
+; Make sure we create the new vp.load in the same place
+define <vscale x 8 x i16> @bitcast_vp_load_insertpoint(ptr %p) {
+; CHECK-LABEL: define <vscale x 8 x i16> @bitcast_vp_load_insertpoint(
+; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0]] {
+; CHECK-NEXT:    [[R:%.*]] = call <vscale x 8 x i16> @llvm.vp.load.nxv8i16.p0(ptr align 16 [[P]], <vscale x 8 x i1> splat (i1 true), i32 4)
+; CHECK-NEXT:    store i32 0, ptr [[P]], align 4
+; CHECK-NEXT:    ret <vscale x 8 x i16> [[R]]
+;
+  %l = call <vscale x 4 x i32> @llvm.vp.load(ptr %p, <vscale x 4 x i1> splat (i1 true), i32 2)
+  store i32 0, ptr %p
+  %r = bitcast <vscale x 4 x i32> %l to <vscale x 8 x i16>
+  ret <vscale x 8 x i16> %r
+}

[dyung]

The commit that commit that motivated the fix that you are attempting to backport here doesn't seem to be present on the release branch. Does that mean we still need the fix?

[tru]

@lukel97 friendly ping on @dyung's question above — is the load-reordering bug fixed here actually reachable on release/23.x, given the motivating commit (93ac788) isn't on the branch? If it's not reachable, this backport may not be needed post-Final.

[lukel97]

@dyung @tru sorry I missed your mention, yes it's reachable on main even without 93ac788. That commit just caused the miscompile to be hit with Clang specifically. It can be reached with regular LLVM IR which might affect downstream vectorizers