[llvm] 931b606 - [BasicAA] Handle assumes with operand bundles

Nikita Popov via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 24 10:04:36 PDT 2021


On Wed, Mar 24, 2021 at 1:10 AM Philip Reames <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>
> 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 match what
you have in mind here?

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
>> > 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
>> > 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/c654f286/attachment.html>


More information about the llvm-commits mailing list