[PATCH] D84609: [MemDepAnalysis] Cut-off threshold reshuffling

Thu Aug 6 13:42:53 PDT 2020

asbirlea added a comment.

In D84609#2182442 <https://reviews.llvm.org/D84609#2182442>, @nikic wrote:

> In D84609#2180272 <https://reviews.llvm.org/D84609#2180272>, @asbirlea wrote:
>
>> In D84609#2180222 <https://reviews.llvm.org/D84609#2180222>, @lebedev.ri wrote:
>>
>>> I wasn't really asking about NewGVN story, i know it's stagnant somewhat.
>>> I was only asking, would it be better to instead look into porting
>>> `GVN::processNonLocalLoad()` to be MemorySSA-driven.
>>
>> If we're building two analyses (MSSA & MemDepAnalysis) instead of one, I expect we'll see a spike in compile-time. Additionally, MemDepAnalysis is a piece of technical debt that would be nice to replace altogether.
>> IMO it makes sense to make the transition for the whole pass, or for the pipeline of 3 passes in one go.
>
> I recently ran some numbers for GVN compile-time impact: http://llvm-compile-time-tracker.com/index.php?branch=nikic/perf/new-gvn-3 From the bottom to the top, the first commit disables LoadPRE, the second disables the entirety of processNonLocalLoad and the last one enables NewGVN (with the corresponding MemorySSA run).
>
> I think the key takeaways here is that the non-local load analysis in GVN is really, really expensive. Per-block MemDep analysis is fairly cheap, but the non-local one is not, and I think GVN is the only MemDep-based pass that uses it.
>
> So, if it is feasible to replace the non-local load analysis (and load PRE) in GVN with something MemorySSA based, I think it's plausible that it will be a compile-time improvement despite the need to construct MemorySSA. And it would become free for a following MemCpyOpt/DSE :)
>
> Of course, this is on the assumption that MemorySSA can actually perform a similar degree of optimization with better compile-time. As these optimizations require walking past MemoryPhis, it's not going to be free, but I would still expect it to be cheaper and have better cutoff control.

Thank you for this analysis, this is excellent information!
I'll need to look in detail into what `processNonLocalLoad` does to see if MemorySSA can do better there; noted as the next priority.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84609