[PATCH] "float2int": Add a new pass to demote from float to int where possible.

[hfinkel at anl.gov]

REPOSITORY
  rL LLVM

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:48
@@ +47,3 @@
+static cl::opt<unsigned>
+MaxIntegerBW("-float2int-max-integer-bw", cl::init(64), cl::Hidden,
+             cl::desc("Max integer bitwidth to consider in float2int"
----------------
Don't put the "-" at the beginning of "-float2int-max-integer-bw".

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:65
@@ +64,3 @@
+    void findRoots(Function &F, SmallPtrSet<Instruction*,8> &Roots);
+    ConstantRange Seen(Instruction *I, ConstantRange R);
+    ConstantRange BadRange();
----------------
Please make all of the functions here start with a lowercase letter.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:74
@@ +73,3 @@
+
+    MapVector<Instruction*, ConstantRange > SeenInsts;
+    SmallPtrSet<Instruction*,8> Roots;
----------------
Remove extra space before >

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:164
@@ +163,3 @@
+// up into two phases:
+//   - walkForwards:   A breadth-first walk of the use-def graph starting from
+//                     the roots. Populate "SeenInsts" with interesting
----------------
I don't like the terminology of calling this a 'forward' walk. It is walking instructions from the roots (which are at the end of the execution sequence), through their operands. I call this a 'backward' walk.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:169
@@ +168,3 @@
+//                     while we're here too.
+//   - walkBackwards:  Iterate over SeenInsts in reverse order, so we visit
+//                     defs before their uses. Calculate the real range info.
----------------
If you visit defs before uses, that seems 'forward' to me.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:230
@@ +229,3 @@
+    }
+
+  }
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:295
@@ +294,3 @@
+      if (Instruction *OI = dyn_cast<Instruction>(O)) {
+
+        assert(SeenInsts.find(OI) != SeenInsts.end() &&
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:299
@@ +298,3 @@
+        OpRanges.push_back(SeenInsts.find(OI)->second);
+
+      } else if (ConstantFP *CF = dyn_cast<ConstantFP>(O)) {
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:301
@@ +300,3 @@
+      } else if (ConstantFP *CF = dyn_cast<ConstantFP>(O)) {
+
+        // Work out if the floating point number can be losslessly represented
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:338
@@ +337,3 @@
+        OpRanges.push_back(ConstantRange(Int));
+
+      } else {
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:347
@@ +346,3 @@
+      Seen(I, Op(OpRanges));
+    
+  }
----------------
Remove blank line.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:418
@@ +417,3 @@
+    // FIXME: Pick the smallest legal type that will fit.
+    Type *Ty = (MinBW > 32) ? Type::getInt64Ty(*Ctx) : Type::getInt32Ty(*Ctx);
+
----------------
Where do you filter out MinBW > 64?

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:421
@@ +420,3 @@
+    for (auto MI = ECs.member_begin(It), ME = ECs.member_end();
+         MI != ME; ++MI) {
+      convert(*MI, Ty);
----------------
{ } not needed here.

================
Comment at: lib/Transforms/Scalar/Float2Int.cpp:472
@@ +471,3 @@
+    assert(P != CmpInst::BAD_ICMP_PREDICATE && "Unhandled predicate!");
+    NewV = IRB.CreateICmp(P,
+                          NewOperands[0], NewOperands[1],
----------------
Unnecessary line break?

================
Comment at: test/Transforms/Float2Int/basic.ll:119
@@ +118,3 @@
+; Negative tests
+;
+
----------------
Please add negative test using a large integer type (i128, etc.).

http://reviews.llvm.org/D7790

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/