[FunctionAttrs] Treat byval calls as only reading ptrs (#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.

Author: AlexMaclean

llvm/include/llvm/IR/InstrTypes.h

@@ -1667,6 +1667,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);
   }
 

llvm/lib/Analysis/AliasAnalysis.cpp

@@ -636,9 +636,7 @@ ModRefInfo AAResults::callCapturesBefore(const Instruction *I,
     // Only look at the no-capture or byval pointer arguments.  If this
     // pointer were passed to arguments that were neither of these, then it
     // couldn't be no-capture.
-    if (!(*CI)->getType()->isPointerTy() ||
-        (!Call->doesNotCapture(ArgNo) && ArgNo < Call->arg_size() &&
-         !Call->isByValArgument(ArgNo)))
+    if (!(*CI)->getType()->isPointerTy() || !Call->doesNotCapture(ArgNo))
       continue;
 
     AliasResult AR =

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

@@ -852,7 +852,7 @@ 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);
 
@@ -890,11 +890,14 @@ 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)) {
+      } else if (!isModSet(ArgMR) || CB.onlyReadsMemory(UseIndex) || IsByVal) {
         IsRead = true;
       } else if (!isRefSet(ArgMR) ||
                  CB.dataOperandHasImpliedAttr(UseIndex, Attribute::WriteOnly)) {

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 nocapture readonly [[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: {{.*}}

llvm/test/Transforms/MemCpyOpt/memcpy-byval-forwarding-clobbers.ll

@@ -11,19 +11,15 @@ declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture)
 declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture)
 declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)
 
-; %a.2's lifetime ends before the call to @check. Cannot replace
-; %a.1 with %a.2 in the call to @check.
+; %a.2's lifetime ends before the call to @check. We must remove the call to
+; @llvm.lifetime.end in order to replace %a.1 with %a.2 in the call to @check.
 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]]