[PATCH] D18407: Add -fno-jump-tables and-fjump-tables flags

[Hans Wennborg via llvm-commits]

hans added a comment.

Besides a few nits, this looks good as far as I can tell. Eric should probably sign off on it, though.


================
Comment at: include/clang/Driver/Options.td:590
@@ +589,3 @@
+def fjump_tables : Flag<["-"], "fjump-tables">, Group<f_Group>,
+  HelpText<"Use jump tables for sufficiently large switch">;
+def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group<f_Group>, Flags<[CC1Option]>;
----------------
It's not just the size of the switch, but also the density, etc.

I think it would make more sense to add HelpText to the -fno flag below instead. How about:

"Do not use jump tables for lowering switches"

================
Comment at: lib/CodeGen/CodeGenFunction.cpp:715
@@ +714,3 @@
+  // Add no-jump-tables value
+  Fn->addFnAttr("no-jump-tables", llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
+
----------------
nit: period at the end of comment, and 80+ column line

================
Comment at: test/CodeGen/nousejumptable.c:3
@@ +2,3 @@
+
+//CHECK: Function Attrs: nounwind nousejumptable
+//CHECK-LABEL: @main()
----------------
ultra nit: there's usually a space between the // and CHECK

================
Comment at: test/CodeGen/nousejumptable.c:6
@@ +5,3 @@
+
+volatile static unsigned choice;
+void ret_A();
----------------
Is there a need for volatile here? Otherwise, drop it.


http://reviews.llvm.org/D18407