[PATCH] D14064: Convert Returned Constant i1 Values to i32 on PPC64

[Kit Barton via llvm-commits]

kbarton added a comment.

A couple general comments, in addition to the inline comments below:

1. Could you please add the LLVM::Statistics class to collect some stats on how many times we are changing bools to ints
2. Are there any pass dependencies that need to be specified for this pass?


================
Comment at: include/llvm/LinkAllPasses.h:68
@@ -67,2 +67,3 @@
       (void) llvm::createBoundsCheckingPass();
+       (void) llvm::createBoolRetToIntPass();
       (void) llvm::createBreakCriticalEdgesPass();
----------------
Spacing is off here

================
Comment at: lib/Target/PowerPC/PPCTargetMachine.cpp:286
@@ -285,1 +285,3 @@
+  if (TM->getOptLevel() != CodeGenOpt::None)
+    addPass(createBoolRetToIntPass());
   addPass(createAtomicExpandPass(&getPPCTargetMachine()));
----------------
This placement will make this one of the first passes that gets runs, won't it? Is this where we want to run it? I admit I haven't thought through the different placements for this, but for some reason I was expecting it to run later (not sure why). 

================
Comment at: lib/Transforms/Scalar/BoolRetToInt.cpp:130
@@ +129,3 @@
+
+    PHINodeSet PromotablePHINodes = getPromotablePHINodes(F);
+    bool Changed = false;
----------------
Why not make PromotablePHINodes a private member of BoolRetToInt? 
That saves the copy here, and eliminates the need to pass it to the other methods. 


http://reviews.llvm.org/D14064