[PATCH] D11856: LowerBitSets: Extend pass to support functions as bitset members.

[JF Bastien via llvm-commits]

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

A few comments, but lgtm overall. It would be nice to get another review on this as it's somewhat tricky stuff :-)


================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:591
@@ +590,3 @@
+unsigned LowerBitSets::getJumpTableEntrySize() {
+  if (Arch != Triple::x86 && Arch != Triple::x86_64)
+    report_fatal_error("Unsupported architecture for jump tables");
----------------
That sounds fine, though the documentation should be updated the the appropriate x86 limitations, and potentially evolution out of this.

================
Comment at: lib/Transforms/IPO/LowerBitSets.cpp:654
@@ +653,3 @@
+/// Given a disjoint set of bitsets and functions, build a jump table for the
+/// functions, build the bit sets and lower the llvm.bitset.test calls.
+/// The function array must be non-empty (the buildBitSetsFromGlobalVariables
----------------
Could you just assert at the function entry? Code tends to evolve past comments ;-)


http://reviews.llvm.org/D11856