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

[Matt Denton via Phabricator via llvm-commits]

mpdenton added a comment.

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?

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?).

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)

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.

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)


Repository:
  rL LLVM

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

https://reviews.llvm.org/D66987