[PATCH] D112313: [DSE] Optimize defining access of defs while walking upwards.

Thu Nov 11 17:18:48 PST 2021

asbirlea added a comment.

In D112313#3125662 <https://reviews.llvm.org/D112313#3125662>, @fhahn wrote:

> In D112313#3125182 <https://reviews.llvm.org/D112313#3125182>, @asbirlea wrote:
>
>> Allowing passes to do this is a slippery slope...
>> We already have the issue that it is sometimes hard to reproduce an issue with a single pass, due to the cached state in MemorySSA. This cached state is already dependent on what other passes do, a mix between queries and transforms, which may leave MemorySSA either "over-optimized" (has stored info beyond what it could deduce if built from scratch) or "under-optimized" (could deduce more). Having DSE optimize more is not that far fetched considering this.
>> I'm inclined to let DSE do this, only because 1) the traversals and inferences are beyond what MSSA alone does now and 2) they're "free" (i.e. DSE does them anyway).
>> However, I would not be ok with any pass being able to set optimized accesses.
>
> That policy sounds good to me, thanks for sharing! Should we add something like that to the MemorySSA docs?

Yes. I'll send something for review tomorrow.

>> Actionable feedback for this patch: can you introduce a cl::opt flag and set `CanOptimize = flag && current_conditions`, and document what the flag does and why.
>> Set the flag to false until there is a concrete case where this will be used (does it further affect the results in D112315 <https://reviews.llvm.org/D112315>?).
>> Have a separate patch that just turn flag to true and evaluate that.
>
> I added a flag in the latest version.

Changes LG.  @nikic?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112313