[llvm] [IR] Treat calls with byval ptrs as read-only (PR #122961)

Alex MacLean via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 14 12:08:33 PST 2025


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

None

>From 93321805f5d0ba5abff7e212d6110aebaef592b2 Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Tue, 14 Jan 2025 20:04:46 +0000
Subject: [PATCH 1/2] pre-commit tests

---
 llvm/test/Analysis/BasicAA/call-attrs.ll       |  4 ++++
 llvm/test/Transforms/SROA/readonlynocapture.ll | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/llvm/test/Analysis/BasicAA/call-attrs.ll b/llvm/test/Analysis/BasicAA/call-attrs.ll
index c42c908310746d..a9f95e4ac3b079 100644
--- a/llvm/test/Analysis/BasicAA/call-attrs.ll
+++ b/llvm/test/Analysis/BasicAA/call-attrs.ll
@@ -3,6 +3,7 @@
 declare void @readonly_attr(ptr readonly nocapture)
 declare void @writeonly_attr(ptr writeonly nocapture)
 declare void @readnone_attr(ptr readnone nocapture)
+declare void @byval_attr(ptr byval(i32))
 
 declare void @readonly_func(ptr nocapture) readonly
 declare void @writeonly_func(ptr nocapture) writeonly
@@ -24,6 +25,8 @@ entry:
   call void @readnone_attr(ptr %p)
   call void @readnone_func(ptr %p)
 
+  call void @byval_attr(ptr %p)
+
   call void @read_write(ptr %p, ptr %p, ptr %p)
 
   call void @func() ["deopt" (ptr %p)]
@@ -38,6 +41,7 @@ entry:
 ; CHECK:  Just Mod:  Ptr: i8* %p	<->  call void @writeonly_func(ptr %p)
 ; CHECK:  NoModRef:  Ptr: i8* %p	<->  call void @readnone_attr(ptr %p)
 ; CHECK:  NoModRef:  Ptr: i8* %p	<->  call void @readnone_func(ptr %p)
+; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @byval_attr(ptr %p)
 ; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @read_write(ptr %p, ptr %p, ptr %p)
 ; CHECK:  Just Ref:  Ptr: i8* %p	<->  call void @func() [ "deopt"(ptr %p) ]
 ; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @writeonly_attr(ptr %p) [ "deopt"(ptr %p) ]
diff --git a/llvm/test/Transforms/SROA/readonlynocapture.ll b/llvm/test/Transforms/SROA/readonlynocapture.ll
index 2c21624f3ea51a..bfb831ad170be4 100644
--- a/llvm/test/Transforms/SROA/readonlynocapture.ll
+++ b/llvm/test/Transforms/SROA/readonlynocapture.ll
@@ -390,4 +390,21 @@ define i32 @testcallalloca() {
   ret i32 %l1
 }
 
+declare void @callee_byval(ptr byval(i32) %p)
+
+define i32 @simple_byval() {
+; CHECK-LABEL: @simple_byval(
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    store i32 0, ptr [[A]], align 4
+; CHECK-NEXT:    call void @callee_byval(ptr [[A]])
+; CHECK-NEXT:    [[L1:%.*]] = load i32, ptr [[A]], align 4
+; CHECK-NEXT:    ret i32 [[L1]]
+;
+  %a = alloca i32
+  store i32 0, ptr %a
+  call void @callee_byval(ptr %a)
+  %l1 = load i32, ptr %a
+  ret i32 %l1
+}
+
 declare void @llvm.memcpy.p0.p0.i64(ptr, ptr, i64, i1)

>From 0e472f86879125e14fe00218ed332431bc8b2c22 Mon Sep 17 00:00:00 2001
From: Alex Maclean <amaclean at nvidia.com>
Date: Tue, 14 Jan 2025 20:07:24 +0000
Subject: [PATCH 2/2] [IR] Treat calls with byval ptrs as read-only

---
 llvm/include/llvm/IR/InstrTypes.h                            | 5 +++++
 llvm/lib/Transforms/IPO/FunctionAttrs.cpp                    | 5 +----
 .../Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp    | 5 -----
 llvm/test/Analysis/BasicAA/call-attrs.ll                     | 2 +-
 llvm/test/Transforms/SROA/readonlynocapture.ll               | 3 +--
 5 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/llvm/include/llvm/IR/InstrTypes.h b/llvm/include/llvm/IR/InstrTypes.h
index b8d9cc10292f4a..47ddc7555594c5 100644
--- a/llvm/include/llvm/IR/InstrTypes.h
+++ b/llvm/include/llvm/IR/InstrTypes.h
@@ -1719,6 +1719,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 onlyReadsMemory(unsigned OpNo) const {
+    // If the argument is passed byval, the callee does not have access to the
+    // original pointer and thus cannot write to it.
+    if (OpNo < arg_size() && isByValArgument(OpNo))
+      return true;
+
     return dataOperandHasImpliedAttr(OpNo, Attribute::ReadOnly) ||
            dataOperandHasImpliedAttr(OpNo, Attribute::ReadNone);
   }
diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
index 06b5d791abe95e..3256e975a846bc 100644
--- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
+++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
@@ -890,14 +890,11 @@ 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)) {
         /* nop */
-      } else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex) || IsByVal) {
+      } else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex)) {
         IsRead = true;
       } else if (!isRefSet(ArgMR) ||
                  CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 93d183837d6f43..f87a4a58470402 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -112,11 +112,6 @@ isOnlyCopiedFromConstantMemory(AAResults *AA, AllocaInst *V,
         if ((Call->onlyReadsMemory() && (Call->use_empty() || NoCapture)) ||
             (Call->onlyReadsMemory(DataOpNo) && NoCapture))
           continue;
-
-        // If this is being passed as a byval argument, the caller is making a
-        // copy, so it is only a read of the alloca.
-        if (IsArgOperand && Call->isByValArgument(DataOpNo))
-          continue;
       }
 
       // Lifetime intrinsics can be handled by the caller.
diff --git a/llvm/test/Analysis/BasicAA/call-attrs.ll b/llvm/test/Analysis/BasicAA/call-attrs.ll
index a9f95e4ac3b079..f6e92dd34ff7fb 100644
--- a/llvm/test/Analysis/BasicAA/call-attrs.ll
+++ b/llvm/test/Analysis/BasicAA/call-attrs.ll
@@ -41,7 +41,7 @@ entry:
 ; CHECK:  Just Mod:  Ptr: i8* %p	<->  call void @writeonly_func(ptr %p)
 ; CHECK:  NoModRef:  Ptr: i8* %p	<->  call void @readnone_attr(ptr %p)
 ; CHECK:  NoModRef:  Ptr: i8* %p	<->  call void @readnone_func(ptr %p)
-; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @byval_attr(ptr %p)
+; CHECK:  Just Ref:  Ptr: i8* %p	<->  call void @byval_attr(ptr %p)
 ; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @read_write(ptr %p, ptr %p, ptr %p)
 ; CHECK:  Just Ref:  Ptr: i8* %p	<->  call void @func() [ "deopt"(ptr %p) ]
 ; CHECK:  Both ModRef:  Ptr: i8* %p	<->  call void @writeonly_attr(ptr %p) [ "deopt"(ptr %p) ]
diff --git a/llvm/test/Transforms/SROA/readonlynocapture.ll b/llvm/test/Transforms/SROA/readonlynocapture.ll
index bfb831ad170be4..611c90ac32b5a2 100644
--- a/llvm/test/Transforms/SROA/readonlynocapture.ll
+++ b/llvm/test/Transforms/SROA/readonlynocapture.ll
@@ -397,8 +397,7 @@ define i32 @simple_byval() {
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    store i32 0, ptr [[A]], align 4
 ; CHECK-NEXT:    call void @callee_byval(ptr [[A]])
-; CHECK-NEXT:    [[L1:%.*]] = load i32, ptr [[A]], align 4
-; CHECK-NEXT:    ret i32 [[L1]]
+; CHECK-NEXT:    ret i32 0
 ;
   %a = alloca i32
   store i32 0, ptr %a



More information about the llvm-commits mailing list