<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p><br>
</p>
<div class="moz-cite-prefix">On 3/24/21 2:12 PM, Nikita Popov wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF+90c8RG1QJ0a=BdmThAcR0_Sc0KqPWp-jgi2DTa_-tk8X6fg@mail.gmail.com">
<meta http-equiv="content-type" content="text/html; charset=UTF-8">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Mar 24, 2021 at
10:06 PM Philip Reames <<a
href="mailto:listmail@philipreames.com"
moz-do-not-send="true">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<p><br>
</p>
<div>On 3/24/21 10:04 AM, Nikita Popov wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Mar 24,
2021 at 1:10 AM Philip Reames <<a
href="mailto:listmail@philipreames.com"
target="_blank" moz-do-not-send="true">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<p><br>
</p>
<div>On 3/23/21 2:35 PM, Nikita Popov wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue,
Mar 23, 2021 at 10:21 PM Philip Reames
<<a
href="mailto:listmail@philipreames.com"
target="_blank" moz-do-not-send="true">listmail@philipreames.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">This
doesn't seem like the right fix. We
conservatively report operand <br>
bundles allowing reads for *unknown*
bundle types. The assume bundle <br>
type should have well defined semantics
and could be explicitly whitelisted.<br>
<br>
Philip<br>
</blockquote>
<div><br>
</div>
<div>I believe the assume operand bundles
are somewhat special and distinct from
other bundles, in that they only apply
to assumes, and in that they are not
explicitly enumerated. An operand bundle
on an assume simply corresponds to the
attribute of the same name.<br>
</div>
</div>
</div>
</blockquote>
Right, that's my point. See
isFnAttrDisallowedByOpBundle and callees on
CallBase. You should be able to simply add
explicit handling for assume bundle there and
everything "should work". <br>
</div>
</blockquote>
<div><br>
</div>
<div>Okay, I think I just misunderstood what you
were suggesting initially. Does <a
href="https://gist.github.com/nikic/0f9733122887065d656d0301e47504a1"
target="_blank" moz-do-not-send="true">https://gist.github.com/nikic/0f9733122887065d656d0301e47504a1</a>
match what you have in mind here?</div>
</div>
</div>
</blockquote>
Yep. I think you've got a slight bug there in that you do
want assumes to access inaccessiblememory, but the general
code structure is exactly what I meant.<br>
</div>
</blockquote>
<div><br>
</div>
Not sure I spot the bug: isFnAttrDisallowedByOpBundle() calls
hasReadingOperandBundles(), which will now return false for
assumes. This means that hasFnAttrOnCalledFunction() gets
called and returns true for the InaccessibleMemOnly attribute.</div>
</div>
</blockquote>
You are correct. I'd missed the last step (that the function decl
was correct.)<br>
<blockquote type="cite"
cite="mid:CAF+90c8RG1QJ0a=BdmThAcR0_Sc0KqPWp-jgi2DTa_-tk8X6fg@mail.gmail.com">
<div dir="ltr">
<div class="gmail_quote"><br>
</div>
<div class="gmail_quote">Regards,<br>
</div>
<div class="gmail_quote">Nikita</div>
<div class="gmail_quote"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<blockquote class="gmail_quote" style="margin:0px
0px 0px 0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex">
<div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div> </div>
<blockquote class="gmail_quote"
style="margin:0px 0px 0px
0.8ex;border-left:1px solid
rgb(204,204,204);padding-left:1ex"> On
3/23/21 1:21 PM, Nikita Popov via
llvm-commits wrote:<br>
> Author: Nikita Popov<br>
> Date: 2021-03-23T21:21:19+01:00<br>
> New Revision:
931b6066acc597e680b8df25521a2f83b40dfaae<br>
><br>
> URL: <a
href="https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae</a><br>
> DIFF: <a
href="https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae.diff"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae.diff</a><br>
><br>
> LOG: [BasicAA] Handle assumes with
operand bundles<br>
><br>
> This fixes a regression reported on
D99022: If a call has operand<br>
> bundles, then the
inaccessiblememonly attribute on the
function<br>
> will be ignored, as operand bundles
can affect modref behavior in<br>
> the general case. However, for
assume operand bundles in particular<br>
> this is not the case.<br>
><br>
> Adjust getModRefBehavior() to
always report inaccessiblememonly<br>
> for assumes, regardless of presence
of operand bundles.<br>
><br>
> Added:<br>
> <br>
><br>
> Modified:<br>
>
llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
>
llvm/test/Analysis/BasicAA/assume.ll<br>
><br>
> Removed:<br>
> <br>
><br>
><br>
>
################################################################################<br>
> diff --git
a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
b/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
> index 86362f770e2d4..65117d82a81ce
100644<br>
> ---
a/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
> +++
b/llvm/lib/Analysis/BasicAliasAnalysis.cpp<br>
> @@ -672,12 +672,22 @@ bool
BasicAAResult::pointsToConstantMemory(const
MemoryLocation &Loc,<br>
> return Worklist.empty();<br>
> }<br>
> <br>
> +static bool isIntrinsicCall(const
CallBase *Call, Intrinsic::ID IID) {<br>
> + const IntrinsicInst *II =
dyn_cast<IntrinsicInst>(Call);<br>
> + return II &&
II->getIntrinsicID() == IID;<br>
> +}<br>
> +<br>
> /// Returns the behavior when
calling the given call site.<br>
> FunctionModRefBehavior
BasicAAResult::getModRefBehavior(const
CallBase *Call) {<br>
> if
(Call->doesNotAccessMemory())<br>
> // Can't do better than this.<br>
> return
FMRB_DoesNotAccessMemory;<br>
> <br>
> + // The assume intrinsic can have
operand bundles, but still only accesses<br>
> + // inaccessible memory in that
case (to maintain control dependencies).<br>
> + if (isIntrinsicCall(Call,
Intrinsic::assume))<br>
> + return
FMRB_OnlyAccessesInaccessibleMem;<br>
> +<br>
> FunctionModRefBehavior Min =
FMRB_UnknownModRefBehavior;<br>
> <br>
> // If the callsite knows it
only reads memory, don't return worse<br>
> @@ -771,11 +781,6 @@ ModRefInfo
BasicAAResult::getArgModRefInfo(const
CallBase *Call,<br>
> return
AAResultBase::getArgModRefInfo(Call,
ArgIdx);<br>
> }<br>
> <br>
> -static bool isIntrinsicCall(const
CallBase *Call, Intrinsic::ID IID) {<br>
> - const IntrinsicInst *II =
dyn_cast<IntrinsicInst>(Call);<br>
> - return II &&
II->getIntrinsicID() == IID;<br>
> -}<br>
> -<br>
> #ifndef NDEBUG<br>
> static const Function
*getParent(const Value *V) {<br>
> if (const Instruction *inst =
dyn_cast<Instruction>(V)) {<br>
><br>
> diff --git
a/llvm/test/Analysis/BasicAA/assume.ll
b/llvm/test/Analysis/BasicAA/assume.ll<br>
> index 4b268836cc49e..bc6be3ef0157e
100644<br>
> ---
a/llvm/test/Analysis/BasicAA/assume.ll<br>
> +++
b/llvm/test/Analysis/BasicAA/assume.ll<br>
> @@ -29,12 +29,12 @@ define void
@test2(i8* %P, i8* %Q) nounwind ssp {<br>
> ; CHECK-LABEL: Function: test2:<br>
> <br>
> ; CHECK: MayAlias: i8* %P, i8*
%Q<br>
> -; CHECK: Both ModRef: Ptr: i8*
%P <-> tail call void
@llvm.assume(i1 true) [ "nonnull"(i8*
%P) ]<br>
> -; CHECK: Both ModRef: Ptr: i8*
%Q <-> tail call void
@llvm.assume(i1 true) [ "nonnull"(i8*
%P) ]<br>
> +; CHECK: NoModRef: Ptr: i8* %P
<-> tail call void
@llvm.assume(i1 true) [ "nonnull"(i8*
%P) ]<br>
> +; CHECK: NoModRef: Ptr: i8* %Q
<-> tail call void
@llvm.assume(i1 true) [ "nonnull"(i8*
%P) ]<br>
> ; CHECK: Both ModRef: Ptr: i8*
%P <-> tail call void
@llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8*
%Q, i64 12, i1 false)<br>
> ; CHECK: Both ModRef: Ptr: i8*
%Q <-> tail call void
@llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8*
%Q, i64 12, i1 false)<br>
> -; CHECK: Both ModRef: tail call
void @llvm.assume(i1 true) [
"nonnull"(i8* %P) ] <-> tail
call void @llvm.memcpy.p0i8.p0i8.i64(i8*
%P, i8* %Q, i64 12, i1 false)<br>
> -; CHECK: Both ModRef: tail call
void @llvm.memcpy.p0i8.p0i8.i64(i8* %P,
i8* %Q, i64 12, i1 false) <->
tail call void @llvm.assume(i1 true) [
"nonnull"(i8* %P) ]<br>
> +; CHECK: NoModRef: tail call
void @llvm.assume(i1 true) [
"nonnull"(i8* %P) ] <-> tail
call void @llvm.memcpy.p0i8.p0i8.i64(i8*
%P, i8* %Q, i64 12, i1 false)<br>
> +; CHECK: NoModRef: tail call
void @llvm.memcpy.p0i8.p0i8.i64(i8* %P,
i8* %Q, i64 12, i1 false) <->
tail call void @llvm.assume(i1 true) [
"nonnull"(i8* %P) ]<br>
> }<br>
> <br>
> attributes #0 = { nounwind }<br>
><br>
><br>
> <br>
>
_______________________________________________<br>
> llvm-commits mailing list<br>
> <a
href="mailto:llvm-commits@lists.llvm.org"
target="_blank" moz-do-not-send="true">llvm-commits@lists.llvm.org</a><br>
> <a
href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits"
rel="noreferrer" target="_blank"
moz-do-not-send="true">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
</div>
</blockquote>
</body>
</html>