[llvm] Unify max alignment for arguments with generic max align. (PR #99257)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 16 16:25:23 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
Author: Eli Friedman (efriedma-quic)
<details>
<summary>Changes</summary>
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.
---
Full diff: https://github.com/llvm/llvm-project/pull/99257.diff
5 Files Affected:
- (modified) llvm/include/llvm/CodeGen/TargetCallingConv.h (+4-4)
- (modified) llvm/lib/IR/Verifier.cpp (+23-15)
- (modified) llvm/test/Verifier/param-align.ll (+7-7)
- (modified) llvm/test/Verifier/param-attr-align.ll (+2-2)
- (modified) llvm/test/Verifier/param-ret-align.ll (+6-6)
``````````diff
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()
``````````
</details>
https://github.com/llvm/llvm-project/pull/99257
More information about the llvm-commits
mailing list