[PATCH] D128830: [Pipelines] Introduce DAE after ArgumentPromotion

[Pavel Samolysov via Phabricator via llvm-commits]

psamolysov added inline comments.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:731
 
-  ModuleInlinerWrapperPass MIWP(
-      IP, PerformMandatoryInliningsFirst,
-      InlineContext{Phase, InlinePass::CGSCCInliner},
-      UseInlineAdvisor, MaxDevirtIterations);
+  ModuleInlinerWrapperPass MIWP(IP, PerformMandatoryInliningsFirst,
+                                InlineContext{Phase, InlinePass::CGSCCInliner},
----------------
fhahn wrote:
> mtrofin wrote:
> > are these changes due to running clang-format (i.e. the previous change didn't?)
> looks like it, usually it's better to use https://github.com/llvm-mirror/clang/blob/master/tools/clang-format/clang-format-diff.py
@mtrofin Yes, this change is the result of a run of clang-format as well as the previous one.

@fhahn Thank you for the suggestion.


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1622
       InlineContext{ThinOrFullLTOPhase::FullLTOPostLink,
-                          InlinePass::CGSCCInliner}));
+                    InlinePass::CGSCCInliner}));
 
----------------
mtrofin wrote:
> same clang-format q
Should I revert the changes introduced by clang-format?


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1689
 
-
   if (EnableConstraintElimination)
----------------
mtrofin wrote:
> spurious change?
This change is introduced by clang-format. Should there be two empty lines between `MainFPM.addPass(MergedLoadStoreMotionPass());` and `if (EnableConstraintElimination)`?


================
Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:1725
   // Run the OpenMPOpt CGSCC pass again late.
-  MPM.addPass(
-      createModuleToPostOrderCGSCCPassAdaptor(OpenMPOptCGSCCPass()));
+  MPM.addPass(createModuleToPostOrderCGSCCPassAdaptor(OpenMPOptCGSCCPass()));
 
----------------
mtrofin wrote:
> was this clang-format?
Yes, it was.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128830