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

via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 15 10:25:59 PST 2025


Author: Alex MacLean
Date: 2025-01-15T10:25:55-08:00
New Revision: 1a56360cc61a3576ab0ad621f72d4299bd5dd0fb

URL: https://github.com/llvm/llvm-project/commit/1a56360cc61a3576ab0ad621f72d4299bd5dd0fb
DIFF: https://github.com/llvm/llvm-project/commit/1a56360cc61a3576ab0ad621f72d4299bd5dd0fb.diff

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

Added: 
    

Modified: 
    llvm/include/llvm/IR/InstrTypes.h
    llvm/lib/Transforms/IPO/FunctionAttrs.cpp
    llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    llvm/test/Analysis/BasicAA/call-attrs.ll
    llvm/test/Analysis/BasicAA/tail-byval.ll
    llvm/test/Transforms/SROA/readonlynocapture.ll

Removed: 
    


################################################################################
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 03cb14c1270c27..56bfc8432cbb2d 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 c42c908310746d..f6e92dd34ff7fb 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:  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/Analysis/BasicAA/tail-byval.ll b/llvm/test/Analysis/BasicAA/tail-byval.ll
index 5c4c563a9a5a81..06c77f44540552 100644
--- a/llvm/test/Analysis/BasicAA/tail-byval.ll
+++ b/llvm/test/Analysis/BasicAA/tail-byval.ll
@@ -12,4 +12,4 @@ entry:
 }
 ; FIXME: This should be Just Ref.
 ; CHECK-LABEL: Function: tailbyval: 1 pointers, 1 call sites
-; CHECK-NEXT:   Both ModRef:  Ptr: i32* %p       <->  tail call void @takebyval(ptr byval(i32) %p)
+; CHECK-NEXT:   Just Ref:  Ptr: i32* %p       <->  tail call void @takebyval(ptr byval(i32) %p)

diff  --git a/llvm/test/Transforms/SROA/readonlynocapture.ll b/llvm/test/Transforms/SROA/readonlynocapture.ll
index 2c21624f3ea51a..611c90ac32b5a2 100644
--- a/llvm/test/Transforms/SROA/readonlynocapture.ll
+++ b/llvm/test/Transforms/SROA/readonlynocapture.ll
@@ -390,4 +390,20 @@ 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:    ret i32 0
+;
+  %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)


        


More information about the llvm-commits mailing list