[PATCH] D22648: [PM] Port NaryReassociate to the new PM

Davide Italiano via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 21 14:57:02 PDT 2016


davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

lgtm but please wait for David


================
Comment at: lib/Transforms/Scalar/NaryReassociate.cpp:162
@@ +161,3 @@
+    return PreservedAnalyses::all();
+  PreservedAnalyses PA;
+  PA.preserve<DominatorTreeAnalysis>();
----------------
Can you add a TODO for setPreservedCFG as other passes do?

================
Comment at: test/Transforms/NaryReassociate/nary-mul.ll:2
@@ -1,2 +1,3 @@
 ; RUN: opt < %s -nary-reassociate -S | FileCheck %s
+; RUN: opt < %s -passes='nary-reassociate' -S | FileCheck %s
 
----------------
wmi wrote:
> davidxl wrote:
> > How about other tests in the same dir?
> I saw the guildeline in https://reviews.llvm.org/rL272505 said one test changed was enough. I just tried other tests and they passed. Do you want me to include them?
I agree that porting a single test is (generally) enough, unless there's a reason to port more for now (e.g. to stress interaction between transform passes and analyses).


Repository:
  rL LLVM

https://reviews.llvm.org/D22648





More information about the llvm-commits mailing list