[llvm] Unify max alignment for arguments with generic max align. (PR #99257)
Eli Friedman via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 17 10:36:53 PDT 2024
https://github.com/efriedma-quic updated https://github.com/llvm/llvm-project/pull/99257
>From 47f034550a5fb9ef6adee6347cd3c00e70ca663d Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Tue, 16 Jul 2024 16:03:38 -0700
Subject: [PATCH 1/2] Unify max alignment for arguments with generic max align.
The 2^14 limit was completely arbitrary; the generic limit is still
arbitrary, but at least it's the same arbitrary limit as everything
else.
While I'm here, also add a verifier check for the ByValOrByRefSize.
---
llvm/include/llvm/CodeGen/TargetCallingConv.h | 8 ++--
llvm/lib/IR/Verifier.cpp | 38 +++++++++++--------
llvm/test/Verifier/param-align.ll | 14 +++----
llvm/test/Verifier/param-attr-align.ll | 4 +-
llvm/test/Verifier/param-ret-align.ll | 12 +++---
5 files changed, 42 insertions(+), 34 deletions(-)
diff --git a/llvm/include/llvm/CodeGen/TargetCallingConv.h b/llvm/include/llvm/CodeGen/TargetCallingConv.h
index 0ff4e1fe657f4..cb0055633f4f3 100644
--- a/llvm/include/llvm/CodeGen/TargetCallingConv.h
+++ b/llvm/include/llvm/CodeGen/TargetCallingConv.h
@@ -45,9 +45,9 @@ namespace ISD {
unsigned IsHva : 1; ///< HVA field for
unsigned IsHvaStart : 1; ///< HVA structure start
unsigned IsSecArgPass : 1; ///< Second argument
- unsigned MemAlign : 4; ///< Log 2 of alignment when arg is passed in memory
- ///< (including byval/byref). The max alignment is
- ///< verified in IR verification.
+ unsigned MemAlign : 6; ///< Log 2 of alignment when arg is passed in memory
+ ///< (including byval/byref). The max alignment is
+ ///< verified in IR verification.
unsigned OrigAlign : 5; ///< Log 2 of original alignment
unsigned IsInConsecutiveRegsLast : 1;
unsigned IsInConsecutiveRegs : 1;
@@ -67,7 +67,7 @@ namespace ISD {
IsSecArgPass(0), MemAlign(0), OrigAlign(0),
IsInConsecutiveRegsLast(0), IsInConsecutiveRegs(0),
IsCopyElisionCandidate(0), IsPointer(0) {
- static_assert(sizeof(*this) == 3 * sizeof(unsigned), "flags are too big");
+ static_assert(sizeof(*this) == 4 * sizeof(unsigned), "flags are too big");
}
bool isZExt() const { return IsZExt; }
diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 75a53c1c99734..c5c407637cbf3 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -324,13 +324,6 @@ namespace {
class Verifier : public InstVisitor<Verifier>, VerifierSupport {
friend class InstVisitor<Verifier>;
-
- // ISD::ArgFlagsTy::MemAlign only have 4 bits for alignment, so
- // the alignment size should not exceed 2^15. Since encode(Align)
- // would plus the shift value by 1, the alignment size should
- // not exceed 2^14, otherwise it can NOT be properly lowered
- // in backend.
- static constexpr unsigned ParamMaxAlignment = 1 << 14;
DominatorTree DT;
/// When verifying a basic block, keep track of all of the
@@ -2021,31 +2014,43 @@ void Verifier::verifyParameterAttrs(AttributeSet Attrs, Type *Ty,
}
if (isa<PointerType>(Ty)) {
+ if (Attrs.hasAttribute(Attribute::Alignment)) {
+ Align AttrAlign = Attrs.getAlignment().valueOrOne();
+ Check(AttrAlign.value() <= Value::MaximumAlignment,
+ "huge alignment values are unsupported", V);
+ }
if (Attrs.hasAttribute(Attribute::ByVal)) {
- if (Attrs.hasAttribute(Attribute::Alignment)) {
- Align AttrAlign = Attrs.getAlignment().valueOrOne();
- Align MaxAlign(ParamMaxAlignment);
- Check(AttrAlign <= MaxAlign,
- "Attribute 'align' exceed the max size 2^14", V);
- }
SmallPtrSet<Type *, 4> Visited;
Check(Attrs.getByValType()->isSized(&Visited),
"Attribute 'byval' does not support unsized types!", V);
+ Check(DL.getTypeAllocSize(Attrs.getByValType()).getKnownMinValue() <
+ (1ULL << 32),
+ "huge 'byval' arguments are unsupported", V);
}
if (Attrs.hasAttribute(Attribute::ByRef)) {
SmallPtrSet<Type *, 4> Visited;
Check(Attrs.getByRefType()->isSized(&Visited),
"Attribute 'byref' does not support unsized types!", V);
+ Check(DL.getTypeAllocSize(Attrs.getByRefType()).getKnownMinValue() <
+ (1ULL << 32),
+ "huge 'byref' arguments are unsupported", V);
}
if (Attrs.hasAttribute(Attribute::InAlloca)) {
SmallPtrSet<Type *, 4> Visited;
Check(Attrs.getInAllocaType()->isSized(&Visited),
"Attribute 'inalloca' does not support unsized types!", V);
+ Check(DL.getTypeAllocSize(Attrs.getInAllocaType()).getKnownMinValue() <
+ (1ULL << 32),
+ "huge 'inalloca' arguments are unsupported", V);
}
if (Attrs.hasAttribute(Attribute::Preallocated)) {
SmallPtrSet<Type *, 4> Visited;
Check(Attrs.getPreallocatedType()->isSized(&Visited),
"Attribute 'preallocated' does not support unsized types!", V);
+ Check(
+ DL.getTypeAllocSize(Attrs.getPreallocatedType()).getKnownMinValue() <
+ (1ULL << 32),
+ "huge 'preallocated' arguments are unsupported", V);
}
}
@@ -3511,12 +3516,15 @@ void Verifier::visitCallBase(CallBase &Call) {
"not allowed. Please use the @llvm.amdgpu.cs.chain intrinsic instead.",
Call);
+ // Disallow passing/returning values with alignment higher than we can
+ // represent.
+ // FIXME: Consider making DataLayout cap the alignment, so this isn't
+ // necessary.
auto VerifyTypeAlign = [&](Type *Ty, const Twine &Message) {
if (!Ty->isSized())
return;
Align ABIAlign = DL.getABITypeAlign(Ty);
- Align MaxAlign(ParamMaxAlignment);
- Check(ABIAlign <= MaxAlign,
+ Check(ABIAlign.value() <= Value::MaximumAlignment,
"Incorrect alignment of " + Message + " to called function!", Call);
};
diff --git a/llvm/test/Verifier/param-align.ll b/llvm/test/Verifier/param-align.ll
index bfd01cbc9faa5..caa8f9ac41ea5 100644
--- a/llvm/test/Verifier/param-align.ll
+++ b/llvm/test/Verifier/param-align.ll
@@ -2,19 +2,19 @@
; Large vector for intrinsics is valid
; CHECK-NOT: llvm.fshr
-define dso_local <8192 x i32> @test_intrin(<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt) {
+define dso_local <2147483648 x i32> @test_intrin(<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt) {
entry:
- %b = call <8192 x i32> @llvm.fshr.v8192i32(<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt)
- ret <8192 x i32> %b
+ %b = call <2147483648 x i32> @llvm.fshr.v8192i32(<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt)
+ ret <2147483648 x i32> %b
}
-declare <8192 x i32> @llvm.fshr.v8192i32 (<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt)
+declare <2147483648 x i32> @llvm.fshr.v8192i32 (<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt)
; CHECK: Incorrect alignment of argument passed to called function!
; CHECK: bar
-define dso_local void @foo(<8192 x float> noundef %vec) {
+define dso_local void @foo(<2147483648 x float> noundef %vec) {
entry:
- call void @bar(<8192 x float> %vec)
+ call void @bar(<2147483648 x float> %vec)
ret void
}
-declare dso_local void @bar(<8192 x float>)
+declare dso_local void @bar(<2147483648 x float>)
diff --git a/llvm/test/Verifier/param-attr-align.ll b/llvm/test/Verifier/param-attr-align.ll
index 038bfa3494f89..700efe5376841 100644
--- a/llvm/test/Verifier/param-attr-align.ll
+++ b/llvm/test/Verifier/param-attr-align.ll
@@ -1,9 +1,9 @@
; RUN: not llvm-as < %s 2>&1 | FileCheck %s
-; CHECK: Attribute 'align' exceed the max size 2^14
+; CHECK: huge alignments are not supported yet
define dso_local void @foo(ptr %p) {
entry:
- call void @bar(ptr noundef byval(<8 x float>) align 32768 %p)
+ call void @bar(ptr noundef byval(<8 x float>) align 8589934592 %p)
ret void
}
diff --git a/llvm/test/Verifier/param-ret-align.ll b/llvm/test/Verifier/param-ret-align.ll
index dd302c38b53d2..98cbb4ee88a89 100644
--- a/llvm/test/Verifier/param-ret-align.ll
+++ b/llvm/test/Verifier/param-ret-align.ll
@@ -2,19 +2,19 @@
; Large vector for intrinsics is valid
; CHECK-NOT: llvm.fshr
-define dso_local <8192 x i32> @test_intrin(<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt) {
+define dso_local <2147483648 x i32> @test_intrin(<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt) {
entry:
- %b = call <8192 x i32> @llvm.fshr.v8192i32(<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt)
- ret <8192 x i32> %b
+ %b = call <2147483648 x i32> @llvm.fshr.v8192i32(<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt)
+ ret <2147483648 x i32> %b
}
-declare <8192 x i32> @llvm.fshr.v8192i32 (<8192 x i32> %l, <8192 x i32> %r, <8192 x i32> %amt)
+declare <2147483648 x i32> @llvm.fshr.v2147483648i32 (<2147483648 x i32> %l, <2147483648 x i32> %r, <2147483648 x i32> %amt)
; CHECK: Incorrect alignment of return type to called function!
; CHECK: bar
define dso_local void @foo() {
entry:
- call <8192 x float> @bar()
+ call <2147483648 x float> @bar()
ret void
}
-declare dso_local <8192 x float> @bar()
+declare dso_local <2147483648 x float> @bar()
>From f206904b62da9f11d982c271f63690a03573ad52 Mon Sep 17 00:00:00 2001
From: Eli Friedman <efriedma at quicinc.com>
Date: Wed, 17 Jul 2024 10:36:30 -0700
Subject: [PATCH 2/2] fixup! add missing test.
---
llvm/test/Verifier/byval-size-limit.ll | 4 ++++
1 file changed, 4 insertions(+)
create mode 100644 llvm/test/Verifier/byval-size-limit.ll
diff --git a/llvm/test/Verifier/byval-size-limit.ll b/llvm/test/Verifier/byval-size-limit.ll
new file mode 100644
index 0000000000000..9b08476453fc9
--- /dev/null
+++ b/llvm/test/Verifier/byval-size-limit.ll
@@ -0,0 +1,4 @@
+; RUN: not llvm-as < %s 2>&1 | FileCheck %s
+
+; CHECK: XXXX
+define void @f(ptr byval([2147483648 x i16])) { ret void }
More information about the llvm-commits
mailing list