[llvm] [FunctionAttrs] Treat byval calls as only reading ptrs (PR #122618)
Alex MacLean via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 12 16:44:44 PST 2025
https://github.com/AlexMaclean updated https://github.com/llvm/llvm-project/pull/122618
>From 8f51b0d74cf469053a6d46eaf0dc4088b0dcafe9 Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Sat, 11 Jan 2025 22:49:59 +0000
Subject: [PATCH 1/4] pre-commit tests
---
.../Transforms/FunctionAttrs/readattrs.ll | 22 +++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index 004c0485d764ae..de5b487580dab9 100644
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -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 [[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: {{.*}}
>From 5ebe920c72022f36484cc6bb9cc94971c07d3429 Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Sat, 11 Jan 2025 22:50:51 +0000
Subject: [PATCH 2/4] [FunctionAttrs] Treat byval calls as only reading ptrs
---
llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 8 +++++---
llvm/test/Transforms/FunctionAttrs/readattrs.ll | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index fe9cca01a8f31f..ca9e97e08ea27e 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -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
@@ -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
@@ -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)) {
diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index de5b487580dab9..6087160d13ca1d 100644
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -766,7 +766,7 @@ declare void @byval_param(ptr byval(i32) %p)
define void @call_byval_param(ptr %p) {
; FNATTRS-LABEL: define {{[^@]+}}@call_byval_param
-; FNATTRS-SAME: (ptr [[P:%.*]]) {
+; FNATTRS-SAME: (ptr readonly [[P:%.*]]) {
; FNATTRS-NEXT: call void @byval_param(ptr byval(i32) [[P]])
; FNATTRS-NEXT: ret void
;
>From 24f49f4b866c6edb72203ff6ad0d41dbe2dfa8a1 Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Sun, 12 Jan 2025 23:41:29 +0000
Subject: [PATCH 3/4] address comments
---
llvm/include/llvm/IR/InstrTypes.h | 5 +++++
llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 7 ++++---
llvm/test/Transforms/FunctionAttrs/readattrs.ll | 2 +-
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index e6332a16df7d5f..adbdbb6e438da6 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -1675,6 +1675,11 @@ class CallBase : public Instruction {
// FIXME: Once this API is no longer duplicated in `CallSite`, rename this to
// better indicate that this may return a conservative answer.
bool doesNotCapture(unsigned OpNo) const {
+ // If the argument is passed byval, the callee does not have access to the
+ // original pointer and thus cannot capture it.
+ if (OpNo < arg_size() && isByValArgument(OpNo))
+ return true;
+
return dataOperandHasImpliedAttr(OpNo, Attribute::NoCapture);
}
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index ca9e97e08ea27e..06b5d791abe95e 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -855,8 +855,6 @@ determinePointerAccessAttrs(Argument *A,
// 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
@@ -866,7 +864,7 @@ determinePointerAccessAttrs(Argument *A,
for (Use &UU : CB.uses())
if (Visited.insert(&UU).second)
Worklist.push_back(&UU);
- } else if (!(CB.doesNotCapture(UseIndex) || IsByVal)) {
+ } else if (!CB.doesNotCapture(UseIndex)) {
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
@@ -892,6 +890,9 @@ determinePointerAccessAttrs(Argument *A,
// can participate in the speculation.
break;
+ const bool IsByVal =
+ CB.isArgOperand(U) && CB.isByValArgument(CB.getArgOperandNo(U));
+
// The accessors used on call site here do the right thing for calls and
// invokes with operand bundles.
if (CB.doesNotAccessMemory(UseIndex)) {
diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index 6087160d13ca1d..e60954c9cd29a3 100644
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -766,7 +766,7 @@ 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-SAME: (ptr nocapture readonly [[P:%.*]]) {
; FNATTRS-NEXT: call void @byval_param(ptr byval(i32) [[P]])
; FNATTRS-NEXT: ret void
;
>From 87890156a71c810a56aebe388b070a00ea698d3a Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Mon, 13 Jan 2025 00:44:32 +0000
Subject: [PATCH 4/4] update test
---
.../MemCpyOpt/memcpy-byval-forwarding-clobbers.ll | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll b/llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
index c28754ebe422fd..6a3dfb3793b15b 100644
--- a/llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
+++ b/llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll
@@ -16,14 +16,10 @@ declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias
define i1 @alloca_forwarding_lifetime_end_clobber() {
; CHECK-LABEL: @alloca_forwarding_lifetime_end_clobber(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[A_1:%.*]] = alloca i64, align 8
; CHECK-NEXT: [[A_2:%.*]] = alloca i64, align 8
-; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[A_2]])
; CHECK-NEXT: call void @init(ptr sret(i64) align 8 [[A_2]])
; CHECK-NEXT: store i8 0, ptr [[A_2]], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr [[A_1]], ptr [[A_2]], i64 8, i1 false)
-; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 8, ptr [[A_2]])
-; CHECK-NEXT: [[CALL:%.*]] = call i1 @check(ptr byval(i64) align 8 [[A_1]])
+; CHECK-NEXT: [[CALL:%.*]] = call i1 @check(ptr byval(i64) align 8 [[A_2]])
; CHECK-NEXT: ret i1 [[CALL]]
;
entry:
@@ -94,13 +90,10 @@ entry:
define i1 @alloca_forwarding_unrelated_call_noclobber() {
; CHECK-LABEL: @alloca_forwarding_unrelated_call_noclobber(
; CHECK-NEXT: entry:
-; CHECK-NEXT: [[A_1:%.*]] = alloca i64, align 8
; CHECK-NEXT: [[A_2:%.*]] = alloca i64, align 8
; CHECK-NEXT: [[A_3:%.*]] = alloca i64, align 8
-; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 8, ptr [[A_2]])
; CHECK-NEXT: call void @init(ptr sret(i64) align 8 [[A_2]])
; CHECK-NEXT: store i8 0, ptr [[A_2]], align 1
-; CHECK-NEXT: call void @llvm.memcpy.p0.p0.i64(ptr [[A_1]], ptr [[A_2]], i64 8, i1 false)
; CHECK-NEXT: call void @clobber(ptr [[A_3]])
; CHECK-NEXT: [[CALL:%.*]] = call i1 @check(ptr byval(i64) align 8 [[A_2]])
; CHECK-NEXT: ret i1 [[CALL]]
More information about the llvm-commits
mailing list