[PATCH] D48239: [LangRef] Clarify meaning of "dereferencable" attribute/metadata.

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D48239#1156604, @efriedma wrote:

> > You're correct about getPointerDereferenceableBytes, but all uses go though isDereferenceableAndAlignedPointer (isDereferenceableAndAlignedPointer is really the only caller of getPointerDereferenceableBytes, as far as I know), and hooking this into isDereferenceableAndAlignedPointer should be straightforward because it takes a context instruction
>
> Well, not all the users pass in the context parameter, since it's optional, but otherwise, that's all we need in theory.


Agreed. Although LICM does, and (I suspect) that's the most important caller.

> That said, it would be too expensive without some sort of cache.  Dereferenceable doesn't imply noalias, so any call could free the pointer.  So we have to iterate over every instruction between the use and the function's entry point to find calls which might alias.  Given we need a cache, we need an analysis pass to store the cache, and that cache has to be preserved by every loop pass so we can use it from LICM.

I'm not sure. One part of this story is that isDereferenceableAndAlignedPointer is itself rarely used directly. LICM calls it directly, as does InstCombine, but essentially all other users call isSafeToLoadUnconditionally which calls isDereferenceableAndAlignedPointer. isSafeToLoadUnconditionally also does scanning for other memory accesses and I've always assumed was not cheap (although I've never profiled it specifically). The scanning is helpful for LICM, so we're right to avoid it there, but my point is that nearly every other place we're querying for this information we're actually doing something relatively expensive.

None of this is to say that we shouldn't be threading a cache of some kind into this interface. We should. It's not clear to me that we need more than a cache of OrderedBasicBlocks (PointerMayBeCapturedBefore can already use one of these), and this is what we currently use with AA.callCapturesBefore. Specifically, I'm thinking of an OrderedInstructions cache (which GVN uses).

I'd be happy to say that we add an `OrderedInstructions *OI = nullptr` to the interface of isDereferenceableAndAlignedPointer, and only use PointerMayBeCapturedBefore if we can provide the associated OBB, otherwise we fallback to calling PointerMayBeCaptured. Adding support in LICM should be relatively straightforward. What do you think?


Repository:
  rL LLVM

https://reviews.llvm.org/D48239