[PATCH] D112313: [DSE] Optimize defining access of defs while walking upwards.
Alina Sbirlea via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
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
More information about the llvm-commits
mailing list