[PATCH] Introduce bitset metadata format and bitset lowering pass.

JF Bastien jfb at chromium.org
Thu Feb 5 10:41:36 PST 2015


I haven't looked at the big function or tests in-depth yet, agreed with @kcc that breaking up the function would make it easier to review.

Could you also add stats to the pass? I'd like Chrome to be able to have a sanity check test that this pass does something.


================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:58
@@ +57,3 @@
+  if (!BitSetTestFunc)
+    return false;
+
----------------
Shouldn't it be an error to try to run the bit set test pass without a module containing the `llvm.bitset.test` intrinsic?

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:75
@@ +74,3 @@
+    if (!CI)
+      continue;
+
----------------
What other valid uses of the intrinsic are there?

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:89
@@ +88,3 @@
+    if (!Ins.second)
+      continue;
+
----------------
I'm not sure I get what's going on here :-)

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:181
@@ +180,3 @@
+      std::vector<uint64_t> Offsets;
+      uint64_t Min = ~0UL, Max = 0;
+
----------------
`UNINT64_MIN` or `std::numeric_limits<uint64_t>::min()` is nicer.

http://reviews.llvm.org/D7288

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






More information about the llvm-commits mailing list