[llvm] 931b606 - [BasicAA] Handle assumes with operand bundles
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 24 14:12:56 PDT 2021
On Wed, Mar 24, 2021 at 10:06 PM Philip Reames <listmail at philipreames.com>
wrote:
>
> 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>
> 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?
>
> 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.
>
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.
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/eb9bcbf1/attachment.html>
More information about the llvm-commits
mailing list