[PATCH] D127547: Add FuncPass Implementation for non-trivial LoopUnswitching

[Sameer Sahasrabuddhe via Phabricator via llvm-commits]

sameerds added a comment.

It seems the loop pass in the new PM already has an option to perform non-trivial unswitching, the purpose of this pass is not clear. Doesn't the core function unswitchLoop() use the same analyses in both cases? Does the function pass use an analysis that is not in the LoopStandardAnalysisResults?



================
Comment at: llvm/lib/Passes/PassRegistry.def:362
 FUNCTION_PASS("sccp", SCCPPass())
+FUNCTION_PASS("simple-loop-unswitch-func", FuncSimpleLoopUnswitchPass())
 FUNCTION_PASS("sink", SinkingPass())
----------------
Is there a better way to name this option? The function pass feels like an implementation detail. Is there something else that conveys the significance?


================
Comment at: llvm/lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:3219-3226
+          auto &Context = L.getHeader()->getContext();
+          MDNode *DisableUnswitchMD = MDNode::get(
+              Context,
+              MDString::get(Context, "llvm.loop.unswitch.partial.disable"));
+          MDNode *NewLoopID = makePostTransformationMetadata(
+              Context, L.getLoopID(), {"llvm.loop.unswitch.partial"},
+              {DisableUnswitchMD});
----------------
The body of this pass closely resembles the loop pass, but the state is too mixed up to refactor. But at least this part with the string literals deserves to be commoned out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127547