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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/MoveAutoInit.cpp:93
+    // The remaining logic should be pretty similar.
+    if (!hasAutoInitMetadata(I))
+      continue;
----------------
serge-sans-paille wrote:
> serge-sans-paille wrote:
> > fhahn wrote:
> > > efriedma wrote:
> > > > Can we avoid iterating over the function if it doesn't have any auto-init?
> > > It's not clear to me from the FIXME why we would want to limit this to `auto-init` instructions. Shouldn't this be beneficial for all memory instructions?
> > > Can we avoid iterating over the function if it doesn't have any auto-init?
> > 
> > I could try a module pass and walk through the users of the auto-init metadata?
> The algorithm beyond auto-init requires more care (it probably needs a reverse transversal of the CFG, and some optimization to avoid rebuilding the dominator of the users for each instruction). I'd rather start small with that pass and then upgrade it to a full-fledge version.
AFAIK the `!annotation` metadata at the moment is information-only and doesn't provide any semantic guarantees. So relying on it for correctness at the moment doesn't sound ideal. From the documentation in the pass, it is not clear what property you are relying on. Could you make this clear in both the documentation in the pass and the commit description?


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

https://reviews.llvm.org/D137707