[PATCH] D29277: [PM] Port ArgumentPromotion to the new pass manager.

Davide Italiano via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 07:07:05 PST 2017


davide added a comment.

I have one high level comment about the tests (it's not your fault but I think it's a good time to recover). I assume you tried to bootstrap and the testsuite with this change and it's clean.



================
Comment at: include/llvm/Analysis/LazyCallGraph.h:234
+    ///
+    /// This is used to facilitate tranfsormations which need to replace the
+    /// formal Function object but directly move the body and users from one to
----------------
s/tranfsormations/transformations/


================
Comment at: lib/Transforms/IPO/ArgumentPromotion.cpp:1081
+
+        // And updat ethe SCC we're iterating as well.
         SCC.ReplaceNode(OldNode, NewNode);
----------------
s/updat e/update/


================
Comment at: test/Transforms/ArgumentPromotion/aggregate-promote.ll:2-4
+; RUN: opt < %s -passes='cgscc(argpromotion,function(instcombine))' -S | not grep load
 target datalayout = "E-p:64:64:64-a0:0:8-f32:32:32-f64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-v64:64:64-v128:128:128"
 
----------------
Ugh for tests still using grep. If you're fine with that, I would rather do the following:
1) Convert them to FileCheck
2) Expand the output so that they don't accidentally test the wrong thing
3) Make sure the output between old and new PM matches.

Maybe I'm being overcautious here, but I've been bitten already by cases where the output didn't quite match (and was hiding bugs).


https://reviews.llvm.org/D29277





More information about the llvm-commits mailing list