[PATCH] D8688: Update MergedLoadStoreMotion to use MemorySSA
Geoff Berry via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 6 08:45:39 PDT 2016
On 7/5/2016 8:21 PM, Daniel Berlin wrote:
> On Wed, Jul 6, 2016 at 1:48 AM, Geoff Berry <gberry at codeaurora.org
> <mailto:gberry at codeaurora.org>> wrote:
> gberry added a comment.
> Sorry if this isn't the right place, but I have a general question
> about what it means to preserve MemorySSA, specifically regarding
> the defining accesses of MemoryUse nodes. Is the idea here that
> we make a best effort to keep the MemoryUse defining access links
> optimized (i.e. never pointing to a no-alias def)?
> Yes. You should fix anything obviously affected/invalidated (this is a
> step past what memdep does, which is mostly nothing). But you should
> not have to redo the links themselves except those related to the
> load/store you touched.
> The *cache* (if any), needs more invalidation, however.
> So if you destroy a load, that means you pretty much have to do nothing.
> If you destroy a store, you either need fine-grained tracking, or
> destroy the entire cache.
Unless I'm missing something, this doesn't jibe with the caching walker
changes under review (http://reviews.llvm.org/D21777). If the walker
doesn't cache MemoryUse clobbering accesses (assuming that a MemoryUse's
defining access is always the "best" clobber), then invalidating the
cache when e.g. destroying a store isn't good enough since the cache is
never consulted when querying about a MemoryUse's clobber.
> Because of the limits of basic-aa, it isn't possible to guarantee
> this property after any code transformation, even in the limited
> case here, since the alias results for completely unrelated
> load/stores may have been affected, right?
> For the moment, yes, but this is 100% brokenness in basicaa that
> should be fixed, and will eventually be fixed.
> There is simply never a good reason to have these limits in basicaa,
> they are symptomatic of basicaa having random stateless backwards
> walkers for properties that can be computed forwards and cached.
> Everything that has such random backwards-walking limits should be
> moved out into a caching pass of some sort that is appropriately
> invalidated or recomputed.
> Lest one worry, they pretty much never need to be recomputed, or at
> most, once (after serious loop optimizations). Nothing changes their
> values because it would change the semantics of the program. Most of
> the time they simply reuse computations but still compute the same end
> result :P
> In any case, your compiler's alias analysis results should not change
> at a basic level simply because you moved from 99 lines of code to 100
> lines (and llvm's is the only one i know which does :( ).
> BasicAA is also being used to do *way* too much. This is a symptom
> of never having any real pointer analysis at all, and so people use
> basicaa to try to figure out pointer analysis problems.
> All of this is a borked design at this point. BasicAA started off
> with good intentions, but it needs a lot of love at this point :)
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the llvm-commits