Skip to content

[FunctionAttrs] Treat byval calls as only reading ptrs #122618

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

Merged
merged 5 commits into from
Jan 13, 2025
Merged

[FunctionAttrs] Treat byval calls as only reading ptrs #122618

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f51b0d
pre-commit tests
AlexMaclean Jan 11, 2025
5ebe920
[FunctionAttrs] Treat byval calls as only reading ptrs
AlexMaclean Jan 11, 2025
24f49f4
address comments
AlexMaclean Jan 12, 2025
8789015
update test
AlexMaclean Jan 13, 2025
89d7648
address comments
AlexMaclean Jan 13, 2025
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
8 changes: 5 additions & 3 deletions llvm/lib/Transforms/IPO/FunctionAttrs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,9 +852,11 @@ determinePointerAccessAttrs(Argument *A,
continue;
}

// Given we've explictily handled the callee operand above, what's left
// Given we've explicitly handled the callee operand above, what's left
// must be a data operand (e.g. argument or operand bundle)
const unsigned UseIndex = CB.getDataOperandNo(U);
const bool IsByVal =
CB.isArgOperand(U) && CB.isByValArgument(CB.getArgOperandNo(U));

// Some intrinsics (for instance ptrmask) do not capture their results,
// but return results thas alias their pointer argument, and thus should
Expand All @@ -864,7 +866,7 @@ determinePointerAccessAttrs(Argument *A,
for (Use &UU : CB.uses())
if (Visited.insert(&UU).second)
Worklist.push_back(&UU);
} else if (!CB.doesNotCapture(UseIndex)) {
} else if (!(CB.doesNotCapture(UseIndex) || IsByVal)) {
if (!CB.onlyReadsMemory())
// If the callee can save a copy into other memory, then simply
// scanning uses of the call is insufficient. We have no way
Expand Down Expand Up @@ -894,7 +896,7 @@ determinePointerAccessAttrs(Argument *A,
// invokes with operand bundles.
if (CB.doesNotAccessMemory(UseIndex)) {
/* nop */
} else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex)) {
} else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex) || IsByVal) {
IsRead = true;
} else if (!isRefSet(ArgMR) ||
CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) {
Expand Down
22 changes: 22 additions & 0 deletions llvm/test/Transforms/FunctionAttrs/readattrs.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,5 +762,27 @@ define void @writable_readnone(ptr writable dereferenceable(4) %p) {
ret void
}

declare void @byval_param(ptr byval(i32) %p)

define void @call_byval_param(ptr %p) {
; FNATTRS-LABEL: define {{[^@]+}}@call_byval_param
; FNATTRS-SAME: (ptr readonly [[P:%.*]]) {
; FNATTRS-NEXT: call void @byval_param(ptr byval(i32) [[P]])
; FNATTRS-NEXT: ret void
;
; ATTRIBUTOR-LABEL: define {{[^@]+}}@call_byval_param
; ATTRIBUTOR-SAME: (ptr nocapture readonly [[P:%.*]]) {
; ATTRIBUTOR-NEXT: call void @byval_param(ptr nocapture readonly byval(i32) [[P]])
; ATTRIBUTOR-NEXT: ret void
;
; ATTRIBUTOR-CGSCC-LABEL: define {{[^@]+}}@call_byval_param
; ATTRIBUTOR-CGSCC-SAME: (ptr nocapture readonly [[P:%.*]]) {
; ATTRIBUTOR-CGSCC-NEXT: call void @byval_param(ptr nocapture readonly byval(i32) [[P]])
; ATTRIBUTOR-CGSCC-NEXT: ret void
;
call void @byval_param(ptr byval(i32) %p)
ret void
}

;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
; COMMON: {{.*}}
Loading