[llvm] 6aa672f - [IR] Take operand bundles into account for call argument readonly/writeonly

[Nikita Popov via llvm-commits]

Author: Nikita Popov
Date: 2022-11-01T09:30:03+01:00
New Revision: 6aa672f14197e84520671de4ce3fcc724bbfe8d9

URL: https://github.com/llvm/llvm-project/commit/6aa672f14197e84520671de4ce3fcc724bbfe8d9
DIFF: https://github.com/llvm/llvm-project/commit/6aa672f14197e84520671de4ce3fcc724bbfe8d9.diff

LOG: [IR] Take operand bundles into account for call argument readonly/writeonly

We currently only take operand bundle effects into account when
querying the function-level memory attributes. However, I believe
that we also need to do the same for parameter attributes. For
example, a call with deopt bundle to a function with readnone
parameter attribute cannot treat that parameter as readnone,
because the deopt bundle may read it.

Differential Revision: https://reviews.llvm.org/D136834

Added: 
    

Modified: 
    llvm/lib/IR/Instructions.cpp
    llvm/test/Analysis/BasicAA/deoptimize.ll
    llvm/test/Transforms/FunctionAttrs/readattrs.ll
    llvm/test/Transforms/FunctionAttrs/writeonly.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp
index f3bcd5322f6a5..8682a938d78c4 100644
--- a/llvm/lib/IR/Instructions.cpp
+++ b/llvm/lib/IR/Instructions.cpp
@@ -344,9 +344,25 @@ bool CallBase::paramHasAttr(unsigned ArgNo, Attribute::AttrKind Kind) const {
 
   if (Attrs.hasParamAttr(ArgNo, Kind))
     return true;
-  if (const Function *F = getCalledFunction())
-    return F->getAttributes().hasParamAttr(ArgNo, Kind);
-  return false;
+
+  const Function *F = getCalledFunction();
+  if (!F)
+    return false;
+
+  if (!F->getAttributes().hasParamAttr(ArgNo, Kind))
+    return false;
+
+  // Take into account mod/ref by operand bundles.
+  switch (Kind) {
+  case Attribute::ReadNone:
+    return !hasReadingOperandBundles() && !hasClobberingOperandBundles();
+  case Attribute::ReadOnly:
+    return !hasClobberingOperandBundles();
+  case Attribute::WriteOnly:
+    return !hasReadingOperandBundles();
+  default:
+    return true;
+  }
 }
 
 bool CallBase::hasFnAttrOnCalledFunction(Attribute::AttrKind Kind) const {

diff  --git a/llvm/test/Analysis/BasicAA/deoptimize.ll b/llvm/test/Analysis/BasicAA/deoptimize.ll
index b8f9f970627b1..6836cc4621e4e 100644
--- a/llvm/test/Analysis/BasicAA/deoptimize.ll
+++ b/llvm/test/Analysis/BasicAA/deoptimize.ll
@@ -21,7 +21,7 @@ define void @test1(i8* %p) {
 ; Check that global G1 is reported as Ref by memcpy/memmove calls.
 define i32 @test_memcpy_with_deopt() {
 ; CHECK-LABEL: Function: test_memcpy_with_deopt:
-; CHECK: Just Mod:  Ptr: i8* %A	<->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
+; CHECK: Both ModRef:  Ptr: i8* %A	<->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 ; CHECK: Just Ref:  Ptr: i8* %B	<->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 ; CHECK: Just Ref:  Ptr: i32* @G1	<->  call void @llvm.memcpy.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 
@@ -40,7 +40,7 @@ define i32 @test_memcpy_with_deopt() {
 
 define i32 @test_memmove_with_deopt() {
 ; CHECK-LABEL: Function: test_memmove_with_deopt:
-; CHECK: Just Mod:  Ptr: i8* %A	<->  call void @llvm.memmove.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
+; CHECK: Both ModRef:  Ptr: i8* %A	<->  call void @llvm.memmove.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 ; CHECK: Just Ref:  Ptr: i8* %B	<->  call void @llvm.memmove.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 ; CHECK: Just Ref:  Ptr: i32* @G1	<->  call void @llvm.memmove.p0i8.p0i8.i64(i8* %A, i8* %B, i64 -1, i1 false) [ "deopt"() ]
 

diff  --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index a982832fa37dd..1833d8b561ccf 100644
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -326,9 +326,10 @@ exit:
 declare void @readnone_param(ptr nocapture readnone %p)
 declare void @readonly_param(ptr nocapture readonly %p)
 
+; FIXME: While this can't be readnone, this could be readonly.
 define void @op_bundle_readnone_deopt(ptr %p) {
 ; CHECK-LABEL: define {{[^@]+}}@op_bundle_readnone_deopt
-; CHECK-SAME: (ptr nocapture readnone [[P:%.*]]) {
+; CHECK-SAME: (ptr nocapture [[P:%.*]]) {
 ; CHECK-NEXT:    call void @readnone_param(ptr [[P]]) [ "deopt"() ]
 ; CHECK-NEXT:    ret void
 ;
@@ -338,7 +339,7 @@ define void @op_bundle_readnone_deopt(ptr %p) {
 
 define void @op_bundle_readnone_unknown(ptr %p) {
 ; CHECK-LABEL: define {{[^@]+}}@op_bundle_readnone_unknown
-; CHECK-SAME: (ptr nocapture readnone [[P:%.*]]) {
+; CHECK-SAME: (ptr nocapture [[P:%.*]]) {
 ; CHECK-NEXT:    call void @readnone_param(ptr [[P]]) [ "unknown"() ]
 ; CHECK-NEXT:    ret void
 ;
@@ -358,7 +359,7 @@ define void @op_bundle_readonly_deopt(ptr %p) {
 
 define void @op_bundle_readonly_unknown(ptr %p) {
 ; CHECK-LABEL: define {{[^@]+}}@op_bundle_readonly_unknown
-; CHECK-SAME: (ptr nocapture readonly [[P:%.*]]) {
+; CHECK-SAME: (ptr nocapture [[P:%.*]]) {
 ; CHECK-NEXT:    call void @readonly_param(ptr [[P]]) [ "unknown"() ]
 ; CHECK-NEXT:    ret void
 ;

diff  --git a/llvm/test/Transforms/FunctionAttrs/writeonly.ll b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
index e4d1347ff9f2b..40ca265990487 100644
--- a/llvm/test/Transforms/FunctionAttrs/writeonly.ll
+++ b/llvm/test/Transforms/FunctionAttrs/writeonly.ll
@@ -180,7 +180,7 @@ define void @direct3b(ptr %p) {
 
 define void @direct3c(ptr %p) {
 ; CHECK-LABEL: define {{[^@]+}}@direct3c
-; CHECK-SAME: (ptr nocapture writeonly [[P:%.*]]) {
+; CHECK-SAME: (ptr nocapture [[P:%.*]]) {
 ; CHECK-NEXT:    call void @direct3_callee(ptr [[P]]) [ "may-read"() ]
 ; CHECK-NEXT:    ret void
 ;