[PATCH] D88365: Port ConstraintElimination to the new pass manager

[Florian Hahn via Phabricator via llvm-commits]

fhahn accepted this revision.
fhahn added a comment.

Thanks for adding NPM support! Not sure if all tests should be also updated to add NPM run-lines.



================
Comment at: llvm/lib/Passes/PassBuilder.cpp:611
 
+  if (EnableConstraintElimination)
+    FPM.addPass(ConstraintEliminationPass());
----------------
MaskRay wrote:
> I add it here to emulate to legacy pass manager builder's behavior.
> 
> I wonder whether the intention is to move some stuff from CorrelatedValuePropagationPass to ConstraintEliminationPass. Does it make sense to place ConstraintEliminationPass before/after CorrelatedValuePropagationPass?
The pass is quite new and not too much experimenting went into finding he ideal position in the pipeline. I expect the position to be adjusted as it evolves. It currently works best when most original conditions are still exposed and branches are used.

There is some overlap with CVP, but the main use case is handling cases that are not handled by CVP (or any other pass).


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:14
 
+#include "llvm/Transforms/Scalar/ConstraintElimination.h"
 #include "llvm/ADT/SmallVector.h"
----------------
IIRC clang-format will sort the includes, so it might be best to move it to the final position?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88365