[PATCH] D66987: [InlineCost] Perform caller store analysis

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

I'm not familiar with the inliner, but I'll try to answer some of the MemorySSA questions.

In D66987#1661697 <https://reviews.llvm.org/D66987#1661697>, @mpdenton wrote:

> So, looking at MemorySSA it looks like getClobberingAccess <https://llvm.org/doxygen/classllvm_1_1MemorySSAWalker.html#a0e395ccb1d5251edf8c1abeafa7aa83a> is the API. I assume the first argument would be the MemoryAccess that corresponds to the callsite, and the MemoryLocation is the caller pointer returned from calleePtrToCallerAllocaOffset. Is there a more sophisticated use here, or do I still need all the restrictions from the current inlining code (e.g. constant offsets)?
>
> Would I need MemorySSA from both the callee and the caller? Is MemorySSA intra-procedural or do I still need to manually perform the translation from caller ptr -> callee ptr with calleePtrToCallerAllocaOffset?


MemorySSA applies to a single function (it is intra-procedural, but not inter-procedural I believe). I am guessing yes to manually perform the translation.

> Also, getClobberingAccess I believe just skips no-alias Defs but can return may-alias. But don't I want something more definitive? I.e. I either want a definitive "yes this writes exactly the memory location you're querying about" or no result at all. So maybe I'd use the walker and then do a manual check to see whether we've found a definitive overwrite. Which would mean I'd be dealing the same edge cases as my current code (overlapping stores?).

Yes. It will also set a "Must" bit for a known must alias. If it set it to May, it's probably not a definitive overwrite.

> Also, if MemorySSA is not intrapodecural, the "search" for stores of constant wouldn't continue past calls anyway, just like my current code. (calls to readonly functions won't show up as clobbers, but my current code handles that already)

That's right, it's not inter-procedural, single function only. Again, not familiar with this. I will guess you could skip more than just readonly calls, but it cannot continue past any call.

> So I'd think MemorySSA would probably buy me "stores from other basic blocks in the function, not just the callsite's basic block", but I'd still probably be dealing with the same edge cases, and MemorySSA brings its own can of worms.

It will probably buy you some more than that but I will guess there will be remaining edge cases and potentially a big can of worms :).

> But I'm new to this MemorySSA thing, so I probably missed something above and there might be some additional benefit to using MemorySSA? (There's the "load elimination" thing from the current inliner code, but I'm considering that orthogonal to this change)

I'll try to look over this in more detail next week to understand the use case. Happy to chat as well.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66987