[PATCH] D127383: Don't treat readnone call in presplit coroutine as not access memory

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:779
+        !Call->hasFnAttr(Attribute::InaccessibleMemOnly) &&
+        !Call->hasFnAttr(Attribute::InaccessibleMemOrArgMemOnly))))
     if (const Function *F = Call->getCalledFunction())
----------------
ChuanqiXu wrote:
> nikic wrote:
> > Do we lose anything substantial with just the `Call->getFunction()->isPresplitCoroutine()` condition?
> > 
> > Alternatively, I would implement this as a fixup afterwards that looks something like this:
> > 
> > ```
> > if (Call->getFunction()->isPreSplitCoroutine())
> >   Min = FunctionModRefBehavior(Min | FMRB_OnlyReadsMemory);
> > ```
> For,
> 
> ```
> define void @f() presplitcoroutine  {
>       entry:
>         %ArgMemOnlyCall = call i32 @argmemonly_func()
>         ret void
>       }
> 
>       declare i32 @argmemonly_func() argmemonly
> ```
> 
> The current implementation would get `FMRB_OnlyAccessesArgumentPointees` for `ArgMemOnlyCall`. But the suggested change would get `FMRB_UnknownModRefBehavior`. I am OK with the suggested change if you feel like the benefit is not worth for the cost. I know compilation time is an important feature of Clang/LLVM.
But isn't FMRB_OnlyAccessesArgumentPointees incorrect, because the thread ID is not an argument, so it accesses non-argument memory?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D127383/new/

https://reviews.llvm.org/D127383