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

Author: AlexMaclean

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);
   }

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)) {

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.

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) ]

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)

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)