Skip to content

[IR][Attributes] Take "best" from Callbase/Call Function when getting attributes #112985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[IR][Attributes] Take "best" from Callbase/Call Function when getting attributes #112985

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
696efd7
[IR][Attributes] Take "best" from Callbase/Call Function when getting…
goldsteinn Oct 18, 2024
3a262f6
Address Feedback
goldsteinn Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 13 additions & 15 deletions llvm/include/llvm/IR/InstrTypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1766,18 +1766,10 @@ class CallBase : public Instruction {
}

/// Extract the alignment of the return value.
MaybeAlign getRetAlign() const {
if (auto Align = Attrs.getRetAlignment())
return Align;
if (const Function *F = getCalledFunction())
return F->getAttributes().getRetAlignment();
return std::nullopt;
}
MaybeAlign getRetAlign() const;

/// Extract the alignment for a call or parameter (0=unknown).
MaybeAlign getParamAlign(unsigned ArgNo) const {
return Attrs.getParamAlignment(ArgNo);
}
MaybeAlign getParamAlign(unsigned ArgNo) const;

MaybeAlign getParamStackAlign(unsigned ArgNo) const {
return Attrs.getParamStackAlignment(ArgNo);
Expand Down Expand Up @@ -1847,7 +1839,11 @@ class CallBase : public Instruction {
/// Extract the number of dereferenceable bytes for a call or
/// parameter (0=unknown).
uint64_t getParamDereferenceableBytes(unsigned i) const {
return Attrs.getParamDereferenceableBytes(i);
uint64_t Bytes = Attrs.getParamDereferenceableBytes(i);
if (const Function *F = getCalledFunction())
Bytes =
std::max(Bytes, F->getAttributes().getParamDereferenceableBytes(i));
return Bytes;
}

/// Extract the number of dereferenceable_or_null bytes for a call
Expand All @@ -1865,7 +1861,11 @@ class CallBase : public Instruction {
/// Extract the number of dereferenceable_or_null bytes for a
/// parameter (0=unknown).
uint64_t getParamDereferenceableOrNullBytes(unsigned i) const {
return Attrs.getParamDereferenceableOrNullBytes(i);
uint64_t Bytes = Attrs.getParamDereferenceableOrNullBytes(i);
if (const Function *F = getCalledFunction())
Bytes = std::max(
Bytes, F->getAttributes().getParamDereferenceableOrNullBytes(i));
return Bytes;
}

/// Extract a test mask for disallowed floating-point value classes for the
Expand All @@ -1886,9 +1886,7 @@ class CallBase : public Instruction {
bool isReturnNonNull() const;

/// Determine if the return value is marked with NoAlias attribute.
bool returnDoesNotAlias() const {
return Attrs.hasRetAttr(Attribute::NoAlias);
}
bool returnDoesNotAlias() const { return hasRetAttr(Attribute::NoAlias); }

/// If one of the arguments has the 'returned' attribute, returns its
/// operand value. Otherwise, return nullptr.
Expand Down
37 changes: 33 additions & 4 deletions llvm/lib/IR/Instructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,39 @@ FPClassTest CallBase::getParamNoFPClass(unsigned i) const {
}

std::optional<ConstantRange> CallBase::getRange() const {
const Attribute RangeAttr = getRetAttr(llvm::Attribute::Range);
if (RangeAttr.isValid())
return RangeAttr.getRange();
return std::nullopt;
Attribute CBRangeAttr = getRetAttr(llvm::Attribute::Range);
Attribute FnRangeAttr{};
if (const Function *F = getCalledFunction())
FnRangeAttr = F->getAttributes().getRetAttr(llvm::Attribute::Range);
if (!CBRangeAttr.isValid() || !FnRangeAttr.isValid()) {
if (CBRangeAttr.isValid())
return CBRangeAttr.getRange();
if (FnRangeAttr.isValid())
return FnRangeAttr.getRange();
return std::nullopt;
}
return CBRangeAttr.getRange().intersectWith(FnRangeAttr.getRange());
}

MaybeAlign CallBase::getRetAlign() const {
MaybeAlign CBAlign = Attrs.getRetAlignment();
MaybeAlign FNAlign = CBAlign;
if (const Function *F = getCalledFunction())
FNAlign = F->getAttributes().getRetAlignment();
if (!CBAlign || !FNAlign)
return !CBAlign ? FNAlign : CBAlign;

return MaybeAlign(std::max(CBAlign.valueOrOne(), FNAlign.valueOrOne()));
}
MaybeAlign CallBase::getParamAlign(unsigned ArgNo) const {
MaybeAlign CBAlign = Attrs.getParamAlignment(ArgNo);
MaybeAlign FNAlign = CBAlign;
if (const Function *F = getCalledFunction())
FNAlign = F->getAttributes().getParamAlignment(ArgNo);
if (!CBAlign || !FNAlign)
return !CBAlign ? FNAlign : CBAlign;

return MaybeAlign(std::max(CBAlign.valueOrOne(), FNAlign.valueOrOne()));
}

bool CallBase::isReturnNonNull() const {
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/AMDGPU/reqd-work-group-size.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ define amdgpu_kernel void @partial_load_group_size_x(ptr addrspace(1) %out) #0 !
; CHECK-LABEL: @partial_load_group_size_x_explicit_callsite_align(
; CHECK-NEXT: %dispatch.ptr = tail call align 2 ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
; CHECK-NEXT: %gep.group.size.x = getelementptr inbounds i8, ptr addrspace(4) %dispatch.ptr, i64 4
; CHECK-NEXT: %group.size.x.lo = load i8, ptr addrspace(4) %gep.group.size.x, align 2
; CHECK-NEXT: %group.size.x.lo = load i8, ptr addrspace(4) %gep.group.size.x, align 4
; CHECK-NEXT: store i8 %group.size.x.lo, ptr addrspace(1) %out, align 1
define amdgpu_kernel void @partial_load_group_size_x_explicit_callsite_align(ptr addrspace(1) %out) #0 !reqd_work_group_size !0 {
%dispatch.ptr = tail call align 2 ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/CodeGen/ARM/ssp-data-layout.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,4 +443,4 @@ declare void @end_struct_large_nonchar()
declare signext i16 @get_struct_small_nonchar()
declare void @end_struct_small_nonchar()

declare void @takes_all(i64, i16, ptr byval(%struct.struct_large_nonchar) align 8, i32, ptr, ptr, ptr, ptr, ptr, i32, i32, i32)
declare void @takes_all(i64, i16, ptr byval(%struct.struct_large_nonchar) align 4, i32, ptr, ptr, ptr, ptr, ptr, i32, i32, i32)
Copy link
Member

@dtcxzyw dtcxzyw Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this change?

Copy link
Contributor Author

@goldsteinn goldsteinn Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the test they always override callsite param with align 4. After this change it switches to 8 which causes ALOT of diffs in the stack allocations. Felt like a mistake when writing the test to have the two diverge.

But probably should add an ARM maintainer to this to be sure.

Copy link
Contributor Author

@goldsteinn goldsteinn Oct 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@davemgreen this okay?

Copy link
Collaborator

@davemgreen davemgreen Nov 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I didn't see this until now. This test is very old now but considering what it does, yes this sounds OK.

28 changes: 14 additions & 14 deletions llvm/test/CodeGen/PowerPC/aix-vec-arg-spills-mir.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ define double @caller() {
; MIR32: bb.0.entry:
; MIR32-NEXT: renamable $r3 = LI 0
; MIR32-NEXT: renamable $r4 = LIS 16392
; MIR32-NEXT: STW killed renamable $r4, 180, $r1 :: (store (s32) into unknown-address + 24)
; MIR32-NEXT: STW killed renamable $r4, 184, $r1 :: (store (s32) into unknown-address + 24, align 8)
; MIR32-NEXT: renamable $r4 = LIS 16384
; MIR32-NEXT: STW renamable $r3, 184, $r1 :: (store (s32) into unknown-address + 28)
; MIR32-NEXT: STW renamable $r3, 176, $r1 :: (store (s32) into unknown-address + 20)
; MIR32-NEXT: STW killed renamable $r4, 172, $r1 :: (store (s32) into unknown-address + 16)
; MIR32-NEXT: STW renamable $r3, 168, $r1 :: (store (s32) into unknown-address + 12)
; MIR32-NEXT: STW renamable $r3, 188, $r1 :: (store (s32) into unknown-address + 28, basealign 8)
; MIR32-NEXT: STW renamable $r3, 180, $r1 :: (store (s32) into unknown-address + 20, basealign 8)
; MIR32-NEXT: STW killed renamable $r4, 176, $r1 :: (store (s32) into unknown-address + 16, align 8)
; MIR32-NEXT: STW renamable $r3, 172, $r1 :: (store (s32) into unknown-address + 12, basealign 8)
; MIR32-NEXT: renamable $r4 = LIS 16368
; MIR32-NEXT: STW killed renamable $r4, 164, $r1 :: (store (s32) into unknown-address + 8)
; MIR32-NEXT: STW renamable $r3, 160, $r1 :: (store (s32) into unknown-address + 4)
; MIR32-NEXT: STW killed renamable $r3, 156, $r1 :: (store (s32))
; MIR32-NEXT: ADJCALLSTACKDOWN 188, 0, implicit-def dead $r1, implicit $r1
; MIR32-NEXT: STW killed renamable $r4, 168, $r1 :: (store (s32) into unknown-address + 8, align 8)
; MIR32-NEXT: STW renamable $r3, 164, $r1 :: (store (s32) into unknown-address + 4, basealign 8)
; MIR32-NEXT: STW killed renamable $r3, 160, $r1 :: (store (s32), align 8)
; MIR32-NEXT: ADJCALLSTACKDOWN 192, 0, implicit-def dead $r1, implicit $r1
; MIR32-NEXT: renamable $vsl0 = XXLXORz
; MIR32-NEXT: renamable $r3 = LI 136
; MIR32-NEXT: renamable $r4 = LI 120
Expand Down Expand Up @@ -73,22 +73,22 @@ define double @caller() {
; MIR32-NEXT: $f12 = XXLXORdpz
; MIR32-NEXT: $f13 = XXLXORdpz
; MIR32-NEXT: BL_NOP <mcsymbol .callee[PR]>, csr_aix32_altivec, implicit-def dead $lr, implicit $rm, implicit $r3, implicit $r4, implicit $f1, implicit killed $f2, implicit killed $v2, implicit killed $v3, implicit killed $v4, implicit killed $v5, implicit killed $v6, implicit killed $v7, implicit killed $v8, implicit killed $v9, implicit killed $v10, implicit killed $v11, implicit killed $v12, implicit killed $v13, implicit killed $f3, implicit killed $f4, implicit killed $f5, implicit killed $f6, implicit killed $f7, implicit killed $f8, implicit killed $f9, implicit killed $f10, implicit killed $f11, implicit killed $f12, implicit killed $f13, implicit $r2, implicit-def $r1, implicit-def $f1
; MIR32-NEXT: ADJCALLSTACKUP 188, 0, implicit-def dead $r1, implicit $r1
; MIR32-NEXT: ADJCALLSTACKUP 192, 0, implicit-def dead $r1, implicit $r1
; MIR32-NEXT: BLR implicit $lr, implicit $rm, implicit $f1
;
; MIR64-LABEL: name: caller
; MIR64: bb.0.entry:
; MIR64-NEXT: renamable $x3 = LI8 2049
; MIR64-NEXT: renamable $x4 = LI8 1
; MIR64-NEXT: renamable $x3 = RLDIC killed renamable $x3, 51, 1
; MIR64-NEXT: STD killed renamable $x3, 216, $x1 :: (store (s64) into unknown-address + 24, align 4)
; MIR64-NEXT: STD killed renamable $x3, 216, $x1 :: (store (s64) into unknown-address + 24)
; MIR64-NEXT: renamable $x3 = LI8 1023
; MIR64-NEXT: renamable $x4 = RLDIC killed renamable $x4, 62, 1
; MIR64-NEXT: STD killed renamable $x4, 208, $x1 :: (store (s64) into unknown-address + 16, align 4)
; MIR64-NEXT: STD killed renamable $x4, 208, $x1 :: (store (s64) into unknown-address + 16)
; MIR64-NEXT: renamable $x4 = LI8 0
; MIR64-NEXT: STD renamable $x4, 192, $x1 :: (store (s64), align 4)
; MIR64-NEXT: STD renamable $x4, 192, $x1 :: (store (s64))
; MIR64-NEXT: renamable $x3 = RLDIC killed renamable $x3, 52, 2
; MIR64-NEXT: STD killed renamable $x3, 200, $x1 :: (store (s64) into unknown-address + 8, align 4)
; MIR64-NEXT: STD killed renamable $x3, 200, $x1 :: (store (s64) into unknown-address + 8)
; MIR64-NEXT: ADJCALLSTACKDOWN 224, 0, implicit-def dead $r1, implicit $r1
; MIR64-NEXT: renamable $vsl0 = XXLXORz
; MIR64-NEXT: renamable $x3 = LI8 160
Expand Down
14 changes: 7 additions & 7 deletions llvm/test/CodeGen/PowerPC/aix-vec-arg-spills.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,16 @@ define double @caller() {
; 32BIT-NEXT: li 3, 0
; 32BIT-NEXT: xxlxor 0, 0, 0
; 32BIT-NEXT: xxlxor 1, 1, 1
; 32BIT-NEXT: stw 4, 180(1)
; 32BIT-NEXT: stw 4, 184(1)
; 32BIT-NEXT: lis 4, 16384
; 32BIT-NEXT: stw 3, 184(1)
; 32BIT-NEXT: stw 3, 176(1)
; 32BIT-NEXT: stw 4, 172(1)
; 32BIT-NEXT: stw 3, 188(1)
; 32BIT-NEXT: stw 3, 180(1)
; 32BIT-NEXT: stw 4, 176(1)
; 32BIT-NEXT: lis 4, 16368
; 32BIT-NEXT: stw 3, 168(1)
; 32BIT-NEXT: stw 3, 172(1)
; 32BIT-NEXT: stw 3, 164(1)
; 32BIT-NEXT: stw 4, 168(1)
; 32BIT-NEXT: stw 3, 160(1)
; 32BIT-NEXT: stw 4, 164(1)
; 32BIT-NEXT: stw 3, 156(1)
; 32BIT-NEXT: li 3, 136
; 32BIT-NEXT: li 4, 120
; 32BIT-NEXT: xxlxor 2, 2, 2
Expand Down
Loading