[PATCH] D106148: [NewPM] Fix wrong perfect forwardings

[Victor Campos via Phabricator via llvm-commits]

vhscampos marked 3 inline comments as done.
vhscampos added inline comments.


================
Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:136
     IsLoopNestPass.push_back(false);
     LoopPasses.emplace_back(new RepeatedLoopPassModelT(std::move(Pass)));
   }
----------------
aeubanks wrote:
> forward?
Pass is an rvalue reference.

Note that the type of 'Pass' is RepeatedPass<PassT>, not PassT, i.e. it does not have the type of the template. Which means that perfect forwarding doesn't work here. Therefore we'd need one implementation for lvalue ref and another for rvalue ref. Since this function is called only passing rvalues, it is safe to have only the rvalue reference version.


================
Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:148
     LoopNestPasses.emplace_back(
         new RepeatedLoopNestPassModelT(std::move(Pass)));
   }
----------------
aeubanks wrote:
> forward?
Same reason as above.


================
Comment at: llvm/include/llvm/Transforms/Scalar/LoopPassManager.h:490
   bool LoopNestMode = (LPM.getNumLoopPasses() == 0);
   return FunctionToLoopPassAdaptor(std::make_unique<PassModelT>(std::move(LPM)),
                                    UseMemorySSA, UseBlockFrequencyInfo,
----------------
aeubanks wrote:
> forward?
In this case here, LPM is fixed as an rvalue reference, so no need for forwarding. That means that it accepts only calls that pass an rvalue. This is true for all call sites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106148