[SROA] Canonicalize homogeneous structs to fixed vectors (opt-in, after memcpyopt) - #165159
Conversation
|
@llvm/pr-subscribers-backend-nvptx @llvm/pr-subscribers-llvm-transforms Author: Yaxun (Sam) Liu (yxsamliu) Changes…te allocas Motivation: SROA would keep temporary allocas (e.g. copies and zero-inits) for homogeneous, 16-byte structs. On targets like AMDGPU these map to scratch memory and can severely hurt performance. The following example could not eliminate the allocas before this change: Method: During rewritePartition, when the slice type is a struct of 2 or 4 identical element types, and DataLayout proves it is tightly packed (no padding; element offsets are iEltSize; StructSize == NEltSize), and the element type is a valid fixed-size vector element, and the total size is at or below a configurable threshold, rewrite the slice type to a fixed vector <N x EltTy>. This runs before the alloca-reuse fast path. Why it works: For tightly packed homogeneous structs, the in-memory representation is bitwise-identical to the corresponding fixed vector, so the transformation is semantics-preserving. The vector form enables SROA/ InstCombine/GVN to replace memcpy/memset and conditional copies with vector selects and a single vector store, allowing the allocas to be eliminated. Tests (flat and nested struct) show allocas/mem* disappear and a <4 x i32> store remains. Control: Introduces -sroa-max-struct-to-vector-bytes=N (default 0 = disabled) to guard the transform by struct size. Enable via:
Full diff: https://github.com/llvm/llvm-project/pull/165159.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp b/llvm/lib/Transforms/Scalar/SROA.cpp
index 5c60fad6f91aa..d31aca0338c91 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -122,6 +122,12 @@ namespace llvm {
/// Disable running mem2reg during SROA in order to test or debug SROA.
static cl::opt<bool> SROASkipMem2Reg("sroa-skip-mem2reg", cl::init(false),
cl::Hidden);
+/// Maximum struct size in bytes to canonicalize homogeneous structs to vectors.
+/// 0 disables the transformation to avoid regressions by default.
+static cl::opt<unsigned> SROAMaxStructToVectorBytes(
+ "sroa-max-struct-to-vector-bytes", cl::init(0), cl::Hidden,
+ cl::desc("Max struct size in bytes to canonicalize homogeneous structs to "
+ "fixed vectors (0=disable)"));
extern cl::opt<bool> ProfcheckDisableMetadataFixes;
} // namespace llvm
@@ -5267,6 +5273,58 @@ AllocaInst *SROA::rewritePartition(AllocaInst &AI, AllocaSlices &AS,
if (VecTy)
SliceTy = VecTy;
+ // Canonicalize homogeneous, tightly-packed 2- or 4-field structs to
+ // a fixed-width vector type when the DataLayout proves bitwise identity.
+ // Do this BEFORE the alloca reuse fast-path so that we don't miss
+ // opportunities to vectorize memcpy on allocas whose SliceTy initially
+ // equals the allocated type.
+ if (SROAMaxStructToVectorBytes) {
+ if (auto *STy = dyn_cast<StructType>(SliceTy)) {
+ unsigned NumElts = STy->getNumElements();
+ if (NumElts == 2 || NumElts == 4) {
+ Type *EltTy =
+ STy->getNumElements() > 0 ? STy->getElementType(0) : nullptr;
+ bool IsAllowedElt = false;
+ if (EltTy && VectorType::isValidElementType(EltTy)) {
+ if (auto *IT = dyn_cast<IntegerType>(EltTy))
+ IsAllowedElt = IT->getBitWidth() >= 8;
+ else if (EltTy->isFloatingPointTy())
+ IsAllowedElt = true;
+ }
+ bool AllSame = IsAllowedElt;
+ for (unsigned I = 1; AllSame && I < NumElts; ++I)
+ if (STy->getElementType(I) != EltTy)
+ AllSame = false;
+ if (AllSame) {
+ const StructLayout *SL = DL.getStructLayout(STy);
+ TypeSize EltTS = DL.getTypeAllocSize(EltTy);
+ if (EltTS.isFixed()) {
+ const uint64_t EltSize = EltTS.getFixedValue();
+ if (EltSize >= 1) {
+ const uint64_t StructSize = SL->getSizeInBytes();
+ if (StructSize != 0 &&
+ StructSize <= SROAMaxStructToVectorBytes) {
+ bool TightlyPacked = (StructSize == NumElts * EltSize);
+ if (TightlyPacked) {
+ for (unsigned I = 0; I < NumElts; ++I) {
+ if (SL->getElementOffset(I) != I * EltSize) {
+ TightlyPacked = false;
+ break;
+ }
+ }
+ }
+ if (TightlyPacked) {
+ Type *NewSliceTy = FixedVectorType::get(EltTy, NumElts);
+ SliceTy = NewSliceTy;
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
// Check for the case where we're going to rewrite to a new alloca of the
// exact same type as the original, and with the same access offsets. In that
// case, re-use the existing alloca, but still run through the rewriter to
diff --git a/llvm/test/Transforms/SROA/struct-to-vector.ll b/llvm/test/Transforms/SROA/struct-to-vector.ll
new file mode 100644
index 0000000000000..ceaf8ea435abb
--- /dev/null
+++ b/llvm/test/Transforms/SROA/struct-to-vector.ll
@@ -0,0 +1,311 @@
+; RUN: opt -passes='sroa,gvn,instcombine,simplifycfg' -S \
+; RUN: -sroa-max-struct-to-vector-bytes=16 %s \
+; RUN: | FileCheck %s \
+; RUN: --check-prefixes=FLAT,NESTED,PADDED,NONHOMO,I1,PTR
+%struct.myint4 = type { i32, i32, i32, i32 }
+
+; FLAT-LABEL: define dso_local void @foo_flat(
+; FLAT-NOT: alloca
+; FLAT-NOT: llvm.memcpy
+; FLAT-NOT: llvm.memset
+; FLAT: insertelement <2 x i64>
+; FLAT: bitcast <2 x i64> %{{[^ ]+}} to <4 x i32>
+; FLAT: select i1 %{{[^,]+}}, <4 x i32> zeroinitializer, <4 x i32> %{{[^)]+}}
+; FLAT: store <4 x i32> %{{[^,]+}}, ptr %x, align 16
+; FLAT: ret void
+define dso_local void @foo_flat(ptr noundef %x, i64 %y.coerce0, i64 %y.coerce1, i32 noundef %cond) {
+entry:
+ %y = alloca %struct.myint4, align 16
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.myint4, align 16
+ %zero = alloca %struct.myint4, align 16
+ %data = alloca %struct.myint4, align 16
+ %0 = getelementptr inbounds nuw { i64, i64 }, ptr %y, i32 0, i32 0
+ store i64 %y.coerce0, ptr %0, align 16
+ %1 = getelementptr inbounds nuw { i64, i64 }, ptr %y, i32 0, i32 1
+ store i64 %y.coerce1, ptr %1, align 8
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %temp, ptr align 16 %y, i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 16 %zero, i8 0, i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %2 = load i32, ptr %cond.addr, align 4
+ %tobool = icmp ne i32 %2, 0
+ br i1 %tobool, label %cond.true, label %cond.false
+
+cond.true:
+ br label %cond.end
+
+cond.false:
+ br label %cond.end
+
+cond.end:
+ %cond1 = phi ptr [ %temp, %cond.true ], [ %zero, %cond.false ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %data, ptr align 16 %cond1, i64 16, i1 false)
+ %3 = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %3, ptr align 16 %data, i64 16, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
+%struct.myint4_base_n = type { i32, i32, i32, i32 }
+%struct.myint4_nested = type { %struct.myint4_base_n }
+
+; NESTED-LABEL: define dso_local void @foo_nested(
+; NESTED-NOT: alloca
+; NESTED-NOT: llvm.memcpy
+; NESTED-NOT: llvm.memset
+; NESTED: insertelement <2 x i64>
+; NESTED: bitcast <2 x i64> %{{[^ ]+}} to <4 x i32>
+; NESTED: select i1 %{{[^,]+}}, <4 x i32> zeroinitializer, <4 x i32> %{{[^)]+}}
+; NESTED: store <4 x i32> %{{[^,]+}}, ptr %x, align 16
+; NESTED: ret void
+define dso_local void @foo_nested(ptr noundef %x, i64 %y.coerce0, i64 %y.coerce1, i32 noundef %cond) {
+entry:
+ %y = alloca %struct.myint4_nested, align 16
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.myint4_nested, align 16
+ %zero = alloca %struct.myint4_nested, align 16
+ %data = alloca %struct.myint4_nested, align 16
+ %0 = getelementptr inbounds nuw { i64, i64 }, ptr %y, i32 0, i32 0
+ store i64 %y.coerce0, ptr %0, align 16
+ %1 = getelementptr inbounds nuw { i64, i64 }, ptr %y, i32 0, i32 1
+ store i64 %y.coerce1, ptr %1, align 8
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %temp, ptr align 16 %y, i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 16 %zero, i8 0, i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %2 = load i32, ptr %cond.addr, align 4
+ %tobool = icmp ne i32 %2, 0
+ br i1 %tobool, label %cond.true, label %cond.false
+
+cond.true:
+ br label %cond.end
+
+cond.false:
+ br label %cond.end
+
+cond.end:
+ %cond1 = phi ptr [ %temp, %cond.true ], [ %zero, %cond.false ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %data, ptr align 16 %cond1, i64 16, i1 false)
+ %3 = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 16 %3, ptr align 16 %data, i64 16, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
+
+; PADDED-LABEL: define dso_local void @foo_padded(
+; PADDED: llvm.memcpy
+; PADDED-NOT: store <
+; PADDED: ret void
+%struct.padded = type { i32, i8, i32, i8 }
+define dso_local void @foo_padded(ptr noundef %x, i32 %a0, i8 %a1,
+ i32 %a2, i8 %a3,
+ i32 noundef %cond) {
+entry:
+ %y = alloca %struct.padded, align 4
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.padded, align 4
+ %zero = alloca %struct.padded, align 4
+ %data = alloca %struct.padded, align 4
+ %y_i32_0 = getelementptr inbounds %struct.padded, ptr %y, i32 0, i32 0
+ store i32 %a0, ptr %y_i32_0, align 4
+ %y_i8_1 = getelementptr inbounds %struct.padded, ptr %y, i32 0, i32 1
+ store i8 %a1, ptr %y_i8_1, align 1
+ %y_i32_2 = getelementptr inbounds %struct.padded, ptr %y, i32 0, i32 2
+ store i32 %a2, ptr %y_i32_2, align 4
+ %y_i8_3 = getelementptr inbounds %struct.padded, ptr %y, i32 0, i32 3
+ store i8 %a3, ptr %y_i8_3, align 1
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %temp, ptr align 4 %y,
+ i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 4 %zero, i8 0, i64 16, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %c.pad = load i32, ptr %cond.addr, align 4
+ %tobool.pad = icmp ne i32 %c.pad, 0
+ br i1 %tobool.pad, label %cond.true.pad, label %cond.false.pad
+
+cond.true.pad:
+ br label %cond.end.pad
+
+cond.false.pad:
+ br label %cond.end.pad
+
+cond.end.pad:
+ %cond1.pad = phi ptr [ %temp, %cond.true.pad ], [ %zero, %cond.false.pad ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %data, ptr align 4 %cond1.pad,
+ i64 16, i1 false)
+ %xv.pad = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 4 %xv.pad, ptr align 4 %data,
+ i64 16, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
+
+; NONHOMO-LABEL: define dso_local void @foo_nonhomo(
+; NONHOMO: llvm.memcpy
+; NONHOMO-NOT: store <
+; NONHOMO: ret void
+%struct.nonhomo = type { i32, i64, i32, i64 }
+define dso_local void @foo_nonhomo(ptr noundef %x, i32 %a0, i64 %a1,
+ i32 %a2, i64 %a3,
+ i32 noundef %cond) {
+entry:
+ %y = alloca %struct.nonhomo, align 8
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.nonhomo, align 8
+ %zero = alloca %struct.nonhomo, align 8
+ %data = alloca %struct.nonhomo, align 8
+ %y_i32_0n = getelementptr inbounds %struct.nonhomo, ptr %y, i32 0, i32 0
+ store i32 %a0, ptr %y_i32_0n, align 4
+ %y_i64_1n = getelementptr inbounds %struct.nonhomo, ptr %y, i32 0, i32 1
+ store i64 %a1, ptr %y_i64_1n, align 8
+ %y_i32_2n = getelementptr inbounds %struct.nonhomo, ptr %y, i32 0, i32 2
+ store i32 %a2, ptr %y_i32_2n, align 4
+ %y_i64_3n = getelementptr inbounds %struct.nonhomo, ptr %y, i32 0, i32 3
+ store i64 %a3, ptr %y_i64_3n, align 8
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %temp, ptr align 8 %y,
+ i64 32, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 8 %zero, i8 0, i64 32, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %c.nh = load i32, ptr %cond.addr, align 4
+ %tobool.nh = icmp ne i32 %c.nh, 0
+ br i1 %tobool.nh, label %cond.true.nh, label %cond.false.nh
+
+cond.true.nh:
+ br label %cond.end.nh
+
+cond.false.nh:
+ br label %cond.end.nh
+
+cond.end.nh:
+ %cond1.nh = phi ptr [ %temp, %cond.true.nh ], [ %zero, %cond.false.nh ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %data, ptr align 8 %cond1.nh,
+ i64 32, i1 false)
+ %xv.nh = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %xv.nh, ptr align 8 %data,
+ i64 32, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
+
+; I1-LABEL: define dso_local void @foo_i1(
+; I1-NOT: <4 x i1>
+; I1: ret void
+%struct.i1x4 = type { i1, i1, i1, i1 }
+define dso_local void @foo_i1(ptr noundef %x, i64 %dummy0, i64 %dummy1,
+ i32 noundef %cond) {
+entry:
+ %y = alloca %struct.i1x4, align 1
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.i1x4, align 1
+ %zero = alloca %struct.i1x4, align 1
+ %data = alloca %struct.i1x4, align 1
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 1 %temp, ptr align 1 %y,
+ i64 4, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 1 %zero, i8 0, i64 4, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %c.i1 = load i32, ptr %cond.addr, align 4
+ %tobool.i1 = icmp ne i32 %c.i1, 0
+ br i1 %tobool.i1, label %cond.true.i1, label %cond.false.i1
+
+cond.true.i1:
+ br label %cond.end.i1
+
+cond.false.i1:
+ br label %cond.end.i1
+
+cond.end.i1:
+ %cond1.i1 = phi ptr [ %temp, %cond.true.i1 ], [ %zero, %cond.false.i1 ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 1 %data, ptr align 1 %cond1.i1,
+ i64 4, i1 false)
+ %xv.i1 = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 1 %xv.i1, ptr align 1 %data,
+ i64 4, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
+
+; PTR-LABEL: define dso_local void @foo_ptr(
+; PTR: llvm.memcpy
+; PTR-NOT: <4 x ptr>
+; PTR: ret void
+%struct.ptr4 = type { ptr, ptr, ptr, ptr }
+define dso_local void @foo_ptr(ptr noundef %x, ptr %p0, ptr %p1,
+ ptr %p2, ptr %p3,
+ i32 noundef %cond) {
+entry:
+ %y = alloca %struct.ptr4, align 8
+ %x.addr = alloca ptr, align 8
+ %cond.addr = alloca i32, align 4
+ %temp = alloca %struct.ptr4, align 8
+ %zero = alloca %struct.ptr4, align 8
+ %data = alloca %struct.ptr4, align 8
+ %y_p0 = getelementptr inbounds %struct.ptr4, ptr %y, i32 0, i32 0
+ store ptr %p0, ptr %y_p0, align 8
+ %y_p1 = getelementptr inbounds %struct.ptr4, ptr %y, i32 0, i32 1
+ store ptr %p1, ptr %y_p1, align 8
+ %y_p2 = getelementptr inbounds %struct.ptr4, ptr %y, i32 0, i32 2
+ store ptr %p2, ptr %y_p2, align 8
+ %y_p3 = getelementptr inbounds %struct.ptr4, ptr %y, i32 0, i32 3
+ store ptr %p3, ptr %y_p3, align 8
+ store ptr %x, ptr %x.addr, align 8
+ store i32 %cond, ptr %cond.addr, align 4
+ call void @llvm.lifetime.start.p0(ptr %temp)
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %temp, ptr align 8 %y,
+ i64 32, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %zero)
+ call void @llvm.memset.p0.i64(ptr align 8 %zero, i8 0, i64 32, i1 false)
+ call void @llvm.lifetime.start.p0(ptr %data)
+ %c.ptr = load i32, ptr %cond.addr, align 4
+ %tobool.ptr = icmp ne i32 %c.ptr, 0
+ br i1 %tobool.ptr, label %cond.true.ptr, label %cond.false.ptr
+
+cond.true.ptr:
+ br label %cond.end.ptr
+
+cond.false.ptr:
+ br label %cond.end.ptr
+
+cond.end.ptr:
+ %cond1.ptr = phi ptr [ %temp, %cond.true.ptr ], [ %zero, %cond.false.ptr ]
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %data, ptr align 8 %cond1.ptr,
+ i64 32, i1 false)
+ %xv.ptr = load ptr, ptr %x.addr, align 8
+ call void @llvm.memcpy.p0.p0.i64(ptr align 8 %xv.ptr, ptr align 8 %data,
+ i64 32, i1 false)
+ call void @llvm.lifetime.end.p0(ptr %data)
+ call void @llvm.lifetime.end.p0(ptr %zero)
+ call void @llvm.lifetime.end.p0(ptr %temp)
+ ret void
+}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
ping |
|
ping |
|
ik you didn't ask me to review, but I recently touched the type selection process so I'll take a look this week. In the meantime though, can you (1) rebase (idk why it's not showing merge conflicts, this should have conflicts with my most recent SROA change) and (2) use |
|
Thanks for offering to review! I've rebased onto latest main and fixed the semantic conflict ( |
🪟 Windows x64 Test Results
✅ The build succeeded and all tests passed. |
YonahGoldberg
left a comment
There was a problem hiding this comment.
Overall looks really good! I do think it can be generalized though, but I'm probably fine to merge it after you respond to a couple of my nits and we can generalize it later (unless you want to work on that then go ahead). I'm pretty new to upstream contributions, so let's get some second opinions from the more experienced people you asked to review as well.
c1ef885 to
7988175
Compare
|
✅ With the latest revision this PR passed the LLVM ABI annotation checker. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
When SROA's getTypePartition yields a homogeneous struct (no padding, no
pointers, no i1 fields), canonicalize it to a fixed vector (e.g.
{ i64, i64 } -> <2 x i64>) so allocas can promote through vector load/store
patterns such as std::function swap with three memcpys.
Introduce shouldCanonicalizeHomogeneousStructToVector for the struct-to-vector
fallback after normal promotion paths fail: require a non-splittable
whole-partition use, reject non-splittable sub-element loads, and for i64
homogeneous partitions recover splittable MemIntrinsic transfers only for
interior subaggregates or full-record integer aggregates of at least 32 bytes
(avoiding the broad 16-byte memcpy bucket that reopened llvm-opt-benchmark
regressions).
Add LLVM_DEBUG tracing for partition-type decisions, extend SROA lit coverage,
update DebugInfo SROA FileChecks for struct-to-vector codegen, and regenerate
NVPTX lower-byval-args.ll expectations (@memcpy_to_param) using llc from
llvm-dev Docker.
Fold the fallback decision into tryCanonicalizeStructToVector and drop redundant structural checks and debug noise so the current conservative policy is easier to read and review.
Wrap the LogSelection lambda parameters as requested by clang-format.
Keep the fallback rule focused on simple memory-use shapes so downstream passes can handle case-specific codegen fallout.
Avoid adding a literal undef CHECK and update the NVPTX byval PTX output after the simplified fallback changes the memcpy_to_param store shape.
…nicalization 2-element homogeneous struct sub-partitions (e.g. the [x,y] slice of a 3-element struct, or a struct.two / struct.p field) are now canonicalized to fixed vectors by tryCanonicalizeStructToVector. Update three DebugInfo tests whose CHECK lines expected scalar i64/i32 fragments but now see <2 x i64> / <2 x i32> debug values.
Use {{.*}} instead of literal undef to satisfy the undef deprecator.
Add a `canonicalize-struct-to-vector` option to `SROAOptions`, off by default. Only the late SROA passes in `addVectorPasses` and the two NVPTX legacy-PM SROAs enable it, so canonicalization runs after `MemCpyOptPass`. Running it earlier can hide memcpy chains from memcpyopt or emit wide stores with undef suffix lanes (see the two new SROA tests). With firing gated, the helper is simplified to require memory-intrinsic-only users, and the element-shape rule accepts any homogeneous element count, any integer width, any FP, and integral pointers. Opt in via `opt -passes='sroa<canonicalize-struct-to-vector>'`.
The default `sroa` pass no longer canonicalizes homogeneous struct partitions to fixed vectors, so restore the scalar CHECKs in `sroa-alloca-offset.ll`, `user-memcpy.ll`, and `nullptr.cl` to match what the default pipeline now produces. NVPTX still opts in, so `lower-byval-args.ll` keeps its vector-shaped CHECKs.
Per @arsenm review feedback on PR llvm#165159: drop the redundant "Canonicalize" verb from the pass-option name. Renames the SROAOptions field, the pipeline-parser string ("canonicalize-struct-to-vector" -> "struct-to-vector"), the createSROAPass parameter, all call sites, and the existing RUN-line strings in two SROA tests.
Per @arsenm review feedback on PR llvm#165159: - Rename numeric SSA values (%1, %2, %3) to named values (%dst, %src, %src.tail) so the test reads more clearly and won't churn on future edits. Regenerated CHECK lines via update_test_checks.py. - Drop the auto-emitted "Function Attrs:" comment line above the llvm.memcpy declaration; the attrs already appear on the declare itself.
Per @arsenm review feedback on PR llvm#165159: the test exists to verify that struct-to-vector canonicalization runs only after memcpyopt in the default pipeline, which is a pipeline-configuration concern rather than a SROA-pass unit test. Move it under test/PhaseOrdering/ and switch the single RUN line to "opt -passes='default<O3>'" so it exercises the real pipeline directly instead of a synthetic pass list.
Per @arsenm review feedback on PR llvm#165159: - Rename numeric SSA values (%1, %2, %3) to named values (%dst, %src, %src.tail) so the test reads more clearly and won't churn on future edits. Regenerated CHECK lines via update_test_checks.py. - Drop the auto-emitted "Function Attrs:" comment line above the llvm.memcpy declaration; the attrs already appear on the declare itself.
Per @arsenm review feedback on PR llvm#165159: the test exists to verify that struct-to-vector canonicalization runs only after memcpyopt in the default pipeline, which is a pipeline-configuration concern rather than a SROA-pass unit test. Move it under test/PhaseOrdering/ and switch the single RUN line to "opt -passes='default<O3>'" so it exercises the real pipeline directly instead of a synthetic pass list.
|
Can we rename the pass option to |
Per @YonahGoldberg follow-up on PR llvm#165159: the transformation is expected to extend to array allocas in a follow-up (motivated by issue llvm#164308 for the Julia frontend), so the per-pass option should not be tied to "struct". Renames the SROAOptions field, the pipeline-parser string ("struct-to-vector" -> "aggregate-to-vector"), the createSROAPass parameter, all call sites, and the existing RUN-line strings in the three SROA tests.
done |
|
It is a bit ironic how that a pass named Scalar-Replacement-of-Aggregates just gained a an option for doing Aggregate-Replacement-of-Scalars (more about the awkward naming of the original pass than a concern about the implementation) |
|
I mean it's more like a Vector-Replacement-of-Aggregates right? |
Per @YonahGoldberg follow-up on PR llvm#165159: the transformation is expected to extend to array allocas in a follow-up (motivated by issue llvm#164308 for the Julia frontend), so the per-pass option should not be tied to "struct". Renames the SROAOptions field, the pipeline-parser string ("struct-to-vector" -> "aggregate-to-vector"), the createSROAPass parameter, all call sites, and the existing RUN-line strings in the three SROA tests.
2ebb91c to
867e303
Compare
| @@ -5086,6 +5088,67 @@ bool SROA::presplitLoadsAndStores(AllocaInst &AI, AllocaSlices &AS) { | |||
| return true; | |||
| } | |||
|
|
|||
| /// Try to canonicalize a homogeneous struct partition to a vector type. | |||
| /// | |||
| /// We can do this if all the elements of the struct are the same and tightly | |||
There was a problem hiding this comment.
Consider IR like this:
define void @d2(ptr %c) {
entry:
%e = alloca { i5, i5 }, align 1
call void @llvm.memcpy.p0.p0.i32(ptr align 1 %e, ptr align 1 %c, i32 2, i1 true)
ret void
}
define void @d3(ptr %c) {
entry:
%e = alloca { i5, i5, i5 }, align 1
call void @llvm.memcpy.p0.p0.i32(ptr align 1 %e, ptr align 1 %c, i32 3, i1 true)
ret void
}
The first function wil be changed to use <2 x i5>, but the second if not changed to use <3 x i5>.
Not sure if this is a problem really, but the code comment here talks about restricting to cases when the elements of the struct are being tightly packed. But that is not really what is happening when comparing the size of the struct and the size of the vector.
Maybe there should be a check that DataLayout::typeSizeEqualsStoreSize is true for the element type if we want to restrict this to structs without padding?
| @@ -0,0 +1,389 @@ | |||
| ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 6 | |||
| ; RUN: opt -passes='sroa<aggregate-to-vector>,gvn,instcombine,simplifycfg' -S %s | FileCheck %s | |||
There was a problem hiding this comment.
Why are we also testing gvn,instcombine,simplifycfg here? Is this needed?
SROA sometimes keeps temporary allocas around for homogeneous structs like
{ i32, i32, i32, i32 }because the partition has only memcpy/memset trafficand no scalar typed users to drive vector promotion. On targets like AMDGPU
these allocas turn into scratch memory and hurt performance. This PR adds a
helper
tryCanonicalizeStructToVectorthat converts such a partition to afixed vector type when every non-debug, non-lifetime user is a memory
intrinsic, so the alloca can promote through normal vector load/store paths.
The element-shape rule accepts any homogeneous element count, any integer
width, any FP type, and integral pointer types, as long as the struct is
tightly packed.
Canonicalization is gated behind a new per-pass option
canonicalize-struct-to-vectoronSROAOptions, off by default. Only thelate SROA passes in
addVectorPasses(new PM, non-LTO and FullLTO) and thetwo legacy-PM SROAs in NVPTX enable it, so it always runs after
MemCpyOptPass. Running it earlier can hide memcpy chains that memcpyoptwould otherwise collapse, and can also emit wide stores whose suffix lanes
are undef when only part of a struct was initialized. Both hazards are
covered by new tests
struct-to-vector-before-memcpyopt.llandstruct-to-vector-fp-store-only-tail.ll. The opt-in design and the"after memcpyopt" placement come from @YonahGoldberg's refactor, which
removed every regression reported in earlier benchmark runs (see
dtcxzyw/llvm-opt-benchmark-nightly#306). AMDGPU inherits the new-PM opt-ins
automatically; other targets keep upstream-main SROA behavior unless they
opt in.
Co-authored-by: Yonah Goldberg ygoldberg@nvidia.com