[llvm] 931b606 - [BasicAA] Handle assumes with operand bundles
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 24 14:06:42 PDT 2021
On 3/24/21 10:04 AM, Nikita Popov wrote:
> On Wed, Mar 24, 2021 at 1:10 AM Philip Reames
> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>
>
> On 3/23/21 2:35 PM, Nikita Popov wrote:
>> On Tue, Mar 23, 2021 at 10:21 PM Philip Reames
>> <listmail at philipreames.com <mailto:listmail at philipreames.com>> wrote:
>>
>> This doesn't seem like the right fix. We conservatively
>> report operand
>> bundles allowing reads for *unknown* bundle types. The
>> assume bundle
>> type should have well defined semantics and could be
>> explicitly whitelisted.
>>
>> Philip
>>
>>
>> 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.
> 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".
>
>
> Okay, I think I just misunderstood what you were suggesting initially.
> Does https://gist.github.com/nikic/0f9733122887065d656d0301e47504a1
> <https://gist.github.com/nikic/0f9733122887065d656d0301e47504a1> match
> what you have in mind here?
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.
>
> Nikita
>
>>
>> Regards,
>> Nikita
>>
>> On 3/23/21 1:21 PM, Nikita Popov via llvm-commits wrote:
>> > Author: Nikita Popov
>> > Date: 2021-03-23T21:21:19+01:00
>> > New Revision: 931b6066acc597e680b8df25521a2f83b40dfaae
>> >
>> > URL:
>> https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae
>> <https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae>
>> > DIFF:
>> https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae.diff
>> <https://github.com/llvm/llvm-project/commit/931b6066acc597e680b8df25521a2f83b40dfaae.diff>
>> >
>> > LOG: [BasicAA] Handle assumes with operand bundles
>> >
>> > This fixes a regression reported on D99022: If a call has
>> operand
>> > bundles, then the inaccessiblememonly attribute on the function
>> > will be ignored, as operand bundles can affect modref
>> behavior in
>> > the general case. However, for assume operand bundles in
>> particular
>> > this is not the case.
>> >
>> > Adjust getModRefBehavior() to always report inaccessiblememonly
>> > for assumes, regardless of presence of operand bundles.
>> >
>> > Added:
>> >
>> >
>> > Modified:
>> > llvm/lib/Analysis/BasicAliasAnalysis.cpp
>> > llvm/test/Analysis/BasicAA/assume.ll
>> >
>> > Removed:
>> >
>> >
>> >
>> >
>> ################################################################################
>> > diff --git a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
>> b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
>> > index 86362f770e2d4..65117d82a81ce 100644
>> > --- a/llvm/lib/Analysis/BasicAliasAnalysis.cpp
>> > +++ b/llvm/lib/Analysis/BasicAliasAnalysis.cpp
>> > @@ -672,12 +672,22 @@ bool
>> BasicAAResult::pointsToConstantMemory(const MemoryLocation &Loc,
>> > return Worklist.empty();
>> > }
>> >
>> > +static bool isIntrinsicCall(const CallBase *Call,
>> Intrinsic::ID IID) {
>> > + const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Call);
>> > + return II && II->getIntrinsicID() == IID;
>> > +}
>> > +
>> > /// Returns the behavior when calling the given call site.
>> > FunctionModRefBehavior
>> BasicAAResult::getModRefBehavior(const CallBase *Call) {
>> > if (Call->doesNotAccessMemory())
>> > // Can't do better than this.
>> > return FMRB_DoesNotAccessMemory;
>> >
>> > + // The assume intrinsic can have operand bundles, but
>> still only accesses
>> > + // inaccessible memory in that case (to maintain control
>> dependencies).
>> > + if (isIntrinsicCall(Call, Intrinsic::assume))
>> > + return FMRB_OnlyAccessesInaccessibleMem;
>> > +
>> > FunctionModRefBehavior Min = FMRB_UnknownModRefBehavior;
>> >
>> > // If the callsite knows it only reads memory, don't
>> return worse
>> > @@ -771,11 +781,6 @@ ModRefInfo
>> BasicAAResult::getArgModRefInfo(const CallBase *Call,
>> > return AAResultBase::getArgModRefInfo(Call, ArgIdx);
>> > }
>> >
>> > -static bool isIntrinsicCall(const CallBase *Call,
>> Intrinsic::ID IID) {
>> > - const IntrinsicInst *II = dyn_cast<IntrinsicInst>(Call);
>> > - return II && II->getIntrinsicID() == IID;
>> > -}
>> > -
>> > #ifndef NDEBUG
>> > static const Function *getParent(const Value *V) {
>> > if (const Instruction *inst = dyn_cast<Instruction>(V)) {
>> >
>> > diff --git a/llvm/test/Analysis/BasicAA/assume.ll
>> b/llvm/test/Analysis/BasicAA/assume.ll
>> > index 4b268836cc49e..bc6be3ef0157e 100644
>> > --- a/llvm/test/Analysis/BasicAA/assume.ll
>> > +++ b/llvm/test/Analysis/BasicAA/assume.ll
>> > @@ -29,12 +29,12 @@ define void @test2(i8* %P, i8* %Q)
>> nounwind ssp {
>> > ; CHECK-LABEL: Function: test2:
>> >
>> > ; CHECK: MayAlias: i8* %P, i8* %Q
>> > -; CHECK: Both ModRef: Ptr: i8* %P <-> tail call void
>> @llvm.assume(i1 true) [ "nonnull"(i8* %P) ]
>> > -; CHECK: Both ModRef: Ptr: i8* %Q <-> tail call void
>> @llvm.assume(i1 true) [ "nonnull"(i8* %P) ]
>> > +; CHECK: NoModRef: Ptr: i8* %P <-> tail call void
>> @llvm.assume(i1 true) [ "nonnull"(i8* %P) ]
>> > +; CHECK: NoModRef: Ptr: i8* %Q <-> tail call void
>> @llvm.assume(i1 true) [ "nonnull"(i8* %P) ]
>> > ; CHECK: Both ModRef: Ptr: i8* %P <-> tail call void
>> @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i1 false)
>> > ; CHECK: Both ModRef: Ptr: i8* %Q <-> tail call void
>> @llvm.memcpy.p0i8.p0i8.i64(i8* %P, i8* %Q, i64 12, i1 false)
>> > -; 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)
>> > -; 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) ]
>> > +; 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)
>> > +; 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) ]
>> > }
>> >
>> > attributes #0 = { nounwind }
>> >
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> <mailto:llvm-commits at lists.llvm.org>
>> >
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>> <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210324/bfcdb38a/attachment.html>
More information about the llvm-commits
mailing list