[MemCpyOpt] Handle scalable aggregate types in memmove/memset formation

[preames]

Without this change, the included test cases crash the compiler. I believe this is fallout from the homogenous scalable struct work from a while back; I think we just forgot to update this case.

Likely to fix #80463.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Philip Reames (preames)

Changes

Without this change, the included test cases crash the compiler. I believe this is fallout from the homogenous scalable struct work from a while back; I think we just forgot to update this case.

Likely to fix #80463.

---

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

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp (+2-2)
• (modified) llvm/test/Transforms/MemCpyOpt/vscale-crashes.ll (+38)

diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 805bbe40bd7c7..1036b8ae963a2 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -677,9 +677,9 @@ bool MemCpyOptPass::processStoreOfLoad(StoreInst *SI, LoadInst *LI,
       if (isModSet(AA->getModRefInfo(SI, LoadLoc)))
         UseMemMove = true;
 
-      uint64_t Size = DL.getTypeStoreSize(T);
-
       IRBuilder<> Builder(P);
+      Value *Size = Builder.CreateTypeSize(Builder.getInt64Ty(),
+                                           DL.getTypeStoreSize(T));
       Instruction *M;
       if (UseMemMove)
         M = Builder.CreateMemMove(
diff --git a/llvm/test/Transforms/MemCpyOpt/vscale-crashes.ll b/llvm/test/Transforms/MemCpyOpt/vscale-crashes.ll
index 84b06f6071ff6..42ad92ee03f4d 100644
--- a/llvm/test/Transforms/MemCpyOpt/vscale-crashes.ll
+++ b/llvm/test/Transforms/MemCpyOpt/vscale-crashes.ll
@@ -85,5 +85,43 @@ define void @callslotoptzn(<vscale x 4 x float> %val, ptr %out) {
   ret void
 }
 
+%0 = type { <vscale x 8 x i8> }
+%1 = type { <vscale x 8 x i8>, <vscale x 8 x i8> }
+
+define void @memmove_vector(ptr %a, ptr %b) {
+; CHECK-LABEL: @memmove_vector(
+; CHECK-NEXT:    [[V:%.*]] = load <vscale x 8 x i8>, ptr [[A:%.*]], align 1
+; CHECK-NEXT:    store <vscale x 8 x i8> [[V]], ptr [[B:%.*]], align 1
+; CHECK-NEXT:    ret void
+;
+  %v = load <vscale x 8 x i8>, ptr %a, align 1
+  store <vscale x 8 x i8> %v, ptr %b, align 1
+  ret void
+}
+
+define void @memmove_agg1(ptr %a, ptr %b) {
+; CHECK-LABEL: @memmove_agg1(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i64 @llvm.vscale.i64()
+; CHECK-NEXT:    [[TMP2:%.*]] = mul i64 [[TMP1]], 8
+; CHECK-NEXT:    call void @llvm.memmove.p0.p0.i64(ptr align 1 [[B:%.*]], ptr align 1 [[A:%.*]], i64 [[TMP2]], i1 false)
+; CHECK-NEXT:    ret void
+;
+  %v = load %0, ptr %a, align 1
+  store %0 %v, ptr %b, align 1
+  ret void
+}
+
+define void @memmove_agg2(ptr %a, ptr %b) {
+; CHECK-LABEL: @memmove_agg2(
+; CHECK-NEXT:    [[TMP1:%.*]] = call i64 @llvm.vscale.i64()
+; CHECK-NEXT:    [[TMP2:%.*]] = mul i64 [[TMP1]], 16
+; CHECK-NEXT:    call void @llvm.memmove.p0.p0.i64(ptr align 1 [[B:%.*]], ptr align 1 [[A:%.*]], i64 [[TMP2]], i1 false)
+; CHECK-NEXT:    ret void
+;
+  %v = load %1, ptr %a, align 1
+  store %1 %v, ptr %b, align 1
+  ret void
+}
+
 declare <vscale x 4 x i32> @llvm.experimental.stepvector.nxv4i32()
 declare void @llvm.masked.scatter.nxv4f32.nxv4p0f32(<vscale x 4 x float> , <vscale x 4 x ptr> , i32, <vscale x 4 x i1>)

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff b78b264518e0f341d99a4291cbf24134c7536f6d 22a3d91d6c0c157d3e6529b18ac30292fbb30a97 -- llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp

View the diff from clang-format here.

diff --git a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
index 1036b8ae96..7802926d20 100644
--- a/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
+++ b/llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp
@@ -678,8 +678,8 @@ bool MemCpyOptPass::processStoreOfLoad(StoreInst *SI, LoadInst *LI,
         UseMemMove = true;
 
       IRBuilder<> Builder(P);
-      Value *Size = Builder.CreateTypeSize(Builder.getInt64Ty(),
-                                           DL.getTypeStoreSize(T));
+      Value *Size =
+          Builder.CreateTypeSize(Builder.getInt64Ty(), DL.getTypeStoreSize(T));
       Instruction *M;
       if (UseMemMove)
         M = Builder.CreateMemMove(

[nikic]

LGTM

[asb]

@preames I believe this is a candidate for backporting? (the test case does indeed seem to crash on the release/18.x branch right now).

[preames]

@preames I believe this is a candidate for backporting? (the test case does indeed seem to crash on the release/18.x branch right now).

It could be, but I don't know we need to. It was a fuzzer failure, nothing from a real workload.