[PATCH] D79454: [IR] `byval` arguments cause reads at call sites

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert created this revision.
jdoerfert added reviewers: arsenm, spatel, efriedma, lebedev.ri, fhahn, rnk.
Herald added subscribers: uenoku, bollu, hiraditya, wdng.
Herald added a reviewer: sstefan1.
Herald added a reviewer: uenoku.
Herald added a project: LLVM.

Even if a call site or function is marked `readnone` (or `writeonly`), a
`byval` argument is still "read"/copied at the call site. We now
indicate so in `Instruction::mayReadOrWriteMemory` and remove the
temporary workaround in the Attributor. Test coverage is provided by the
Attributor tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79454

Files:
  llvm/lib/IR/Instruction.cpp
  llvm/lib/Transforms/IPO/Attributor.cpp
  llvm/lib/Transforms/IPO/AttributorAttributes.cpp


Index: llvm/lib/Transforms/IPO/AttributorAttributes.cpp
===================================================================
--- llvm/lib/Transforms/IPO/AttributorAttributes.cpp
+++ llvm/lib/Transforms/IPO/AttributorAttributes.cpp
@@ -5836,14 +5836,9 @@
       for (const Use &UserIUse : UserI->uses())
         Uses.insert(&UserIUse);
 
-    // If UserI might touch memory we analyze the use in detail. We also do that
-    // for readnone call sites when the use is a byval argument because the call
-    // site performs a copy of the memory.
+    // If UserI might touch memory we analyze the use in detail.
     if (UserI->mayReadOrWriteMemory())
       analyzeUseIn(A, U, UserI);
-    else if (CallBase *CB = dyn_cast<CallBase>(UserI))
-      if (CB->isByValArgument(CB->getArgOperandNo(U)))
-      analyzeUseIn(A, U, UserI);
   }
 
   return (AssumedState != getAssumed()) ? ChangeStatus::CHANGED
Index: llvm/lib/Transforms/IPO/Attributor.cpp
===================================================================
--- llvm/lib/Transforms/IPO/Attributor.cpp
+++ llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1687,11 +1687,6 @@
     }
     if (I.mayReadOrWriteMemory())
       FI.RWInsts.push_back(&I);
-    else if (CallBase *CB = dyn_cast<CallBase>(&I)) {
-      // A byval argument on a readnone function is still read at the call site.
-      if (CB->hasByValArgument())
-        FI.RWInsts.push_back(&I);
-    }
   }
 
   if (F.hasFnAttribute(Attribute::AlwaysInline) &&
Index: llvm/lib/IR/Instruction.cpp
===================================================================
--- llvm/lib/IR/Instruction.cpp
+++ llvm/lib/IR/Instruction.cpp
@@ -537,7 +537,9 @@
   case Instruction::Call:
   case Instruction::Invoke:
   case Instruction::CallBr:
-    return !cast<CallBase>(this)->doesNotReadMemory();
+    // A byval argument is read/copied at the call site.
+    return !cast<CallBase>(this)->doesNotReadMemory() ||
+           cast<CallBase>(this)->hasByValArgument();
   case Instruction::Store:
     return !cast<StoreInst>(this)->isUnordered();
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79454.262255.patch
Type: text/x-patch
Size: 2056 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200505/221e9d03/attachment.bin>