[llvm] [FunctionAttrs] Treat byval calls as only reading ptrs (PR #122618)

[Alex MacLean via llvm-commits]

https://github.com/AlexMaclean created https://github.com/llvm/llvm-project/pull/122618

Since byval arguments are passed via a hidden copy of the pointee, they do not have the same semantics as normal pointer arguments. The callee cannot capture or write to the pointer and the copy is a read of the pointer. 

>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/2] 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 ddc39c571bd16b31532f873e2fb459a217975674 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/2] [FunctionAttrs] Treat byval calls as only reading ptrs

---
 llvm/lib/Transforms/IPO/FunctionAttrs.cpp       | 7 ++++---
 llvm/test/Transforms/FunctionAttrs/readattrs.ll | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index fe9cca01a8f31f..7ac9b70f47902c 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -852,9 +852,10 @@ 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.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 +865,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 +895,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
 ;