From 9e51071eaa925ba8e298e03652bc20d379f59931 Mon Sep 17 00:00:00 2001 From: Eli Friedman Date: Thu, 18 Jul 2024 12:32:13 -0700 Subject: [PATCH] [IR] Unify max alignment for arguments with generic max align. (#99257) 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/byval-size-limit.ll | 4 ++ llvm/test/Verifier/param-align.ll | 14 +++---- llvm/test/Verifier/param-attr-align.ll | 4 +- llvm/test/Verifier/param-ret-align.ll | 12 +++--- 6 files changed, 46 insertions(+), 34 deletions(-) create mode 100644 llvm/test/Verifier/byval-size-limit.ll diff --git a/llvm/include/llvm/CodeGen/TargetCallingConv.h b/llvm/include/llvm/CodeGen/TargetCallingConv.h index 0ff4e1fe657f493..cb0055633f4f332 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 75a53c1c9973467..c5c407637cbf347 100644 --- a/llvm/lib/IR/Verifier.cpp +++ b/llvm/lib/IR/Verifier.cpp @@ -324,13 +324,6 @@ namespace { class Verifier : public InstVisitor, VerifierSupport { friend class InstVisitor; - - // 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(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 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 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 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 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/byval-size-limit.ll b/llvm/test/Verifier/byval-size-limit.ll new file mode 100644 index 000000000000000..3eb462b0636363c --- /dev/null +++ b/llvm/test/Verifier/byval-size-limit.ll @@ -0,0 +1,4 @@ +; RUN: not llvm-as < %s 2>&1 | FileCheck %s + +; CHECK: huge 'byval' arguments are unsupported +define void @f(ptr byval([2147483648 x i16])) { ret void } diff --git a/llvm/test/Verifier/param-align.ll b/llvm/test/Verifier/param-align.ll index bfd01cbc9faa504..caa8f9ac41ea50b 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 038bfa3494f8984..700efe537684120 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 dd302c38b53d2e1..98cbb4ee88a893b 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()