Skip to content

Commit

Permalink
[IR] Unify max alignment for arguments with generic max align. (llvm#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
efriedma-quic authored and sgundapa committed Jul 23, 2024
1 parent a361fd3 commit 9e51071
Show file tree
Hide file tree
Showing 6 changed files with 46 additions and 34 deletions.
8 changes: 4 additions & 4 deletions llvm/include/llvm/CodeGen/TargetCallingConv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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; }
Expand Down
38 changes: 23 additions & 15 deletions llvm/lib/IR/Verifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
};

Expand Down
4 changes: 4 additions & 0 deletions llvm/test/Verifier/byval-size-limit.ll
Original file line number Diff line number Diff line change
@@ -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 }
14 changes: 7 additions & 7 deletions llvm/test/Verifier/param-align.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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>)
4 changes: 2 additions & 2 deletions llvm/test/Verifier/param-attr-align.ll
Original file line number Diff line number Diff line change
@@ -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
}

Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Verifier/param-ret-align.ll
Original file line number Diff line number Diff line change
Expand Up @@ -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()

0 comments on commit 9e51071

Please sign in to comment.