[PATCH] D137707: [WIP] Move "auto-init" instructions to the dominator of their users

Fri Nov 18 03:09:45 PST 2022

serge-sans-paille added a comment.

In D137707#3935019 <https://reviews.llvm.org/D137707#3935019>, @efriedma wrote:

>> I'd appreciate pointer to the literature on "sinking pass".
>
> Look for PRE (partial redundancy elimination) of stores or partial dead store elimination.  Maybe start with https://dl.acm.org/doi/10.1145/277650.277659 .  See also D29865 <https://reviews.llvm.org/D29865> .

Thanks.

>> So it appears that this patch made no difference to the size of the .text section.
>
> This isn't fundamentally surprising; you have the same number of stores before and after sinking.

That's my understanding too. @nickdesaulniers does this patch has an impact on some kernel benchmark, maybe one where `-ftrivial-auto-var-init` brought some slowdown?

================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:886
   EarlyFPM.addPass(SimplifyCFGPass());
+  EarlyFPM.addPass(MoveAutoInitPass());
   EarlyFPM.addPass(SROAPass());
----------------
nickdesaulniers wrote:
> Is it possible to skip this pass if `-ftrivial-auto-var-init=zero` wasn't specified?
It would be possible through a module flag or something, but

1. if we're going to extend that pass beyond auto-var-init, that's going to be useless
2. that would mean a greater coupling with the front end

The following shows no big impact on compile time when the flag is not used, so I don't think it's worth the extra step

https://llvm-compile-time-tracker.com/compare.php?from=f71d32a0eea47b3d2bb43d6be15cf09d47ef6971&to=6ea450ef673666ed6b8a8257ca56f093aae7469c&stat=instructions:u

================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:92
+
+    if (!hasAutoInitMetadata(I))
+      continue;
----------------
nickdesaulniers wrote:
> is there a faster way to do this rather than having to scan the metadata of every operand?
Well, there's an early exit on the instruction metadata, I guess that's enough?

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

https://reviews.llvm.org/D137707