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

Thu Nov 11 13:32:39 PST 2021

fhahn updated this revision to Diff 386647.
fhahn added a comment.

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?

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

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112313

Files:
  llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D112313.386647.patch
Type: text/x-patch
Size: 5062 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20211111/73d41bee/attachment.bin>