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

[Alina Sbirlea via Phabricator via llvm-commits]

asbirlea added a comment.

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.

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.

The flag can be used when attempting to reproduce behavior, to determine if the caching from DSE affects results. It's also a quick way to reverse the decision of allowing any pass to do this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112313