[PATCH] D99022: [IR] Mark assume/annotation as InaccessibleMemOnly

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/lib/Analysis/BasicAliasAnalysis.cpp:950
-  // that proper control dependencies are maintained but they never mods any
-  // particular memory location.
   //
----------------
jdoerfert wrote:
> nikic wrote:
> > jdoerfert wrote:
> > > This and the snipped below are still "needed". Inaccessiblemem is not as powerful as this logic.
> > Why is this still needed (we should add a test if so)? MemoryLocation always refers to an accessible location, so I believe we'll always return NoModRef between InaccessibleMemOnly and MemoryLocation.
> > 
> > For the code below I could see that it may still have an effect because with InaccessibleMemOnly, two assumes would still ModRef each other, but I'm not sure if there's any way for that to be practically relevant.
> Good point. I didn't realize the "other operand" is a MemoryLocation. 
Hi,

I'm seeing this code now return ModRef with a llvm.assume where it used to return NoModRef prior to this patch. In the case of a llvm.assume with bundle operand the InaccessibleMemOnly attribute in the llvm.assume declaration is ignored because of https://reviews.llvm.org/source/llvm-github/browse/main/llvm/include/llvm/IR/InstrTypes.h$2262-2265


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99022