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

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked 2 inline comments as done.
MaskRay added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilder.cpp:611
 
+  if (EnableConstraintElimination)
+    FPM.addPass(ConstraintEliminationPass());
----------------
fhahn wrote:
> 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).
Thanks for the reply. I'll keep it as is (having a consistent position with the legacy PM)


================
Comment at: llvm/lib/Transforms/Scalar/ConstraintElimination.cpp:14
 
+#include "llvm/Transforms/Scalar/ConstraintElimination.h"
 #include "llvm/ADT/SmallVector.h"
----------------
fhahn wrote:
> IIRC clang-format will sort the includes, so it might be best to move it to the final position?
This is the final position. The coding standard prefers the main header as the first. clang-format does this (the main header gets the category 0 and has the highest priority).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88365