Add -flookup-tables and -fno-lookup-tables flags

[sgundapa]

Summary

These flags control the lowering of switch statements to lookup tables.
When "-fno-lookup-tables" is passed, the function attribute "no-lookup-tables"
is set to "true"

[joerg]

test/CodeGen/nouselookuptable.c

1

Check positive flag and the default case as well.

6

Just use f(void) or so. main is special enough, so avoiding it in test cases is often a good idea.

[sgundapa]

Made the changes asked by reviewers

[mcrosier]

Adding @echristo and @ddunbar who have been the primary owners of the driver for the past decade or so.

[echristo]

So, what's the overall logical idea behind the need for this option? While I understand that you don't want people just doing this via the -mllvm set of options, but if you're just doing this to provide a temporary option rather than turning it off in the backend I'm not sure what the point is.

Can you elaborate more on why you need to expose this via the front end?

[sgundapa]

The discussion is scattered across these patches https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579.
I will provide a brief summary here:

The idea is to control the generation of data (lookup table) generated from a function, specifically when the user is not expecting it.
For hexagon, there is tightly coupled memory and the customers usually place "text" in it.
For functions, which generate lookup tables, it is very very expensive to read the table from a far away non-TCM data section.
This option will disable the generation of lookup tables at the expense of code bloat. This is really driven by the customers of hexagon backend.

[sgundapa]

This is not going to be a temporary option

[echristo]

In D35577#817267, @sgundapa wrote:

The discussion is scattered across these patches https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579.
I will provide a brief summary here:

The idea is to control the generation of data (lookup table) generated from a function, specifically when the user is not expecting it.
For hexagon, there is tightly coupled memory and the customers usually place "text" in it.
For functions, which generate lookup tables, it is very very expensive to read the table from a far away non-TCM data section.
This option will disable the generation of lookup tables at the expense of code bloat. This is really driven by the customers of hexagon backend.

I don't think we're communicating effectively. Let me try another way:

"Is there any reason why a user of the hexagon backend will ever not want to set these options to a particular value"

in other words:

"Should this just be part of the tuning for the hexagon backend and not options at all"

Thanks.

[sgundapa]

"Should this just be part of the tuning for the hexagon backend and not options at all"

This will be useful to all the backends/archs that support a tightly coupled memory.
AFAIK, hexagon is not the only target that has a TCM.

[kparzysz]

In D35577#817944, @echristo wrote:

"Should this just be part of the tuning for the hexagon backend and not options at all"

The ultimate decision as to what kind of code to generate out of a switch statement belongs to the customer. The reasons to choose one way over another may involve factors that the compiler has no knowledge of (such as details of the target hardware). I don't think we need separate options to control jump tables and lookup tables: one option to enable/disable the use of memory tables would be sufficient.

[sgundapa]

If it is okay for the reviewers, I have no problem using -fno-jump-tables to this effect.
I need to update https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579

[mcrosier]

In D35577#818936, @kparzysz wrote:

In D35577#817944, @echristo wrote:

"Should this just be part of the tuning for the hexagon backend and not options at all"

I don't think we need separate options to control jump tables and lookup tables: one option to enable/disable the use of memory tables would be sufficient.

I don't have an objection to this direction (i.e., disable the lookup-table optimization when -fno-jump-tables is specified), so long as other are in agreement.

[sgundapa]

I am waiting for others to approve/review the decision.

[hans]

In D35577#818956, @mcrosier wrote:

In D35577#818936, @kparzysz wrote:

In D35577#817944, @echristo wrote:

"Should this just be part of the tuning for the hexagon backend and not options at all"

I don't think we need separate options to control jump tables and lookup tables: one option to enable/disable the use of memory tables would be sufficient.

I don't have an objection to this direction (i.e., disable the lookup-table optimization when -fno-jump-tables is specified), so long as other are in agreement.

I'm ok with disabling lookup tables under -fno-jump-tables as well.

[sgundapa]

Thanks. I will make change to this affect

[sgundapa]

The only change that is needed is to disable lookup-tables based on the attribute "no-jump-tables" set by "-fno-jump-tables" clang flag.
It implies that this change is not required.

include/clang/Driver/Options.td

def fmath_errno : Flag<["-"], "fmath-errno">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Require math functions to indicate errors by setting errno">;
 def fno_math_errno : Flag<["-"], "fno-math-errno">, Group<f_Group>;
 def fbracket_depth_EQ : Joined<["-"], "fbracket-depth=">, Group<f_Group>;
 def fsignaling_math : Flag<["-"], "fsignaling-math">, Group<f_Group>;
 def fno_signaling_math : Flag<["-"], "fno-signaling-math">, Group<f_Group>;
 def fjump_tables : Flag<["-"], "fjump-tables">, Group<f_Group>;
 def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Do not use jump tables for lowering switches">;
+def flookup_tables : Flag<["-"], "flookup-tables">, Group<f_Group>;
+def fno_lookup_tables : Flag<["-"], "fno-lookup-tables">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"Do not use lookup tables for lowering switches">;
 
 // Begin sanitizer flags. These should all be core options exposed in all driver
 // modes.
 let Flags = [CC1Option, CoreOption] in {
 
 def fsanitize_EQ : CommaJoined<["-"], "fsanitize=">, Group<f_clang_Group>,
                    MetaVarName<"<check>">,
                    HelpText<"Turn on runtime checks for various forms of undefined "

include/clang/Frontend/CodeGenOptions.def

 CODEGENOPT(SimplifyLibCalls  , 1, 1) ///< Set when -fbuiltin is enabled.
 CODEGENOPT(SoftFloat         , 1, 0) ///< -soft-float.
 CODEGENOPT(StrictEnums       , 1, 0) ///< Optimize based on strict enum definition.
 CODEGENOPT(StrictVTablePointers, 1, 0) ///< Optimize based on the strict vtable pointers
 CODEGENOPT(TimePasses        , 1, 0) ///< Set when -ftime-report is enabled.
 CODEGENOPT(UnrollLoops       , 1, 0) ///< Control whether loops are unrolled.
 CODEGENOPT(RerollLoops       , 1, 0) ///< Control whether loops are rerolled.
 CODEGENOPT(NoUseJumpTables   , 1, 0) ///< Set when -fno-jump-tables is enabled.
+CODEGENOPT(NoUseLookupTables , 1, 0) ///< Set when -fno-lookup-tables is enabled.
 CODEGENOPT(UnsafeFPMath      , 1, 0) ///< Allow unsafe floating point optzns.
 CODEGENOPT(UnwindTables      , 1, 0) ///< Emit unwind tables.
 CODEGENOPT(VectorizeLoop     , 1, 0) ///< Run loop vectorizer.
 CODEGENOPT(VectorizeSLP      , 1, 0) ///< Run SLP vectorizer.
 
   /// Attempt to use register sized accesses to bit-fields in structures, when
   /// possible.
 CODEGENOPT(UseRegisterSizedBitfieldAccess , 1, 0)

lib/CodeGen/CodeGenFunction.cpp

void CodeGenFunction::StartFunction(GlobalDecl GD,
   if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
     if (CGM.getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
       CGM.getOpenMPRuntime().emitDeclareSimdFunction(FD, Fn);
 
   // Add no-jump-tables value.
   Fn->addFnAttr("no-jump-tables",
                 llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
 
+  // Add no-lookup-tables value.
+  Fn->addFnAttr("no-lookup-tables",
+                llvm::toStringRef(CGM.getCodeGenOpts().NoUseLookupTables));
+
   if (getLangOpts().OpenCL) {
     // Add metadata for a kernel function.
     if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
       EmitOpenCLKernelMetadata(FD, Fn);
   }
 
   // If we are checking function types, emit a function type signature as
   // prologue data.

lib/Driver/ToolChains/Clang.cpp

if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
     CmdArgs.push_back(Args.MakeArgString("-warn-stack-size=" + v));
     A->claim();
   }
 
   if (!Args.hasFlag(options::OPT_fjump_tables, options::OPT_fno_jump_tables,
                     true))
     CmdArgs.push_back("-fno-jump-tables");
 
+  if (!Args.hasFlag(options::OPT_flookup_tables, options::OPT_fno_lookup_tables,
+                    true))
+    CmdArgs.push_back("-fno-lookup-tables");
+
   if (!Args.hasFlag(options::OPT_fpreserve_as_comments,
                     options::OPT_fno_preserve_as_comments, true))
     CmdArgs.push_back("-fno-preserve-as-comments");
 
   if (Arg *A = Args.getLastArg(options::OPT_mregparm_EQ)) {
     CmdArgs.push_back("-mregparm");
     CmdArgs.push_back(A->getValue());
   }

lib/Frontend/CompilerInvocation.cpp

Opts.DataSections = Args.hasFlag(OPT_fdata_sections,
                                    OPT_fno_data_sections, false);
   Opts.UniqueSectionNames = Args.hasFlag(OPT_funique_section_names,
                                          OPT_fno_unique_section_names, true);
 
   Opts.MergeFunctions = Args.hasArg(OPT_fmerge_functions);
 
   Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
 
+  Opts.NoUseLookupTables = Args.hasArg(OPT_fno_lookup_tables);
+
   Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
   Opts.EmitSummaryIndex = false;
   if (Arg *A = Args.getLastArg(OPT_flto_EQ)) {
     StringRef S = A->getValue();
     if (S == "thin")
       Opts.EmitSummaryIndex = true;
     else if (S != "full")
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << S;

test/CodeGen/nouselookuptable.c

+// RUN: %clang_cc1 -S -fno-lookup-tables %s -emit-llvm -o - | FileCheck %s
+
+// CHECK-LABEL: main
+// CHECK: attributes #0 = {{.*}}"no-lookup-tables"="true"{{.*}}
+
+int main() {
+  return 0;
+}