From 6aa672f14197e84520671de4ce3fcc724bbfe8d9 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 27 Oct 2022 12:45:08 +0200 Subject: [PATCH] [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 --- llvm/lib/IR/Instructions.cpp | 22 ++++++++++++++++--- llvm/test/Analysis/BasicAA/deoptimize.ll | 4 ++-- .../Transforms/FunctionAttrs/readattrs.ll | 7 +++--- .../Transforms/FunctionAttrs/writeonly.ll | 2 +- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/llvm/lib/IR/Instructions.cpp b/llvm/lib/IR/Instructions.cpp index f3bcd5322f6a..8682a938d78c 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 b8f9f970627b..6836cc4621e4 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 a982832fa37d..1833d8b561cc 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 e4d1347ff9f2..40ca26599048 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 ;