[SHAVE] Pass all -f, -g, -O, -W options through directly to moviCompile.

[chandlerc]

Comments from the peanut gallery...

lib/Driver/Tools.cpp

9482–9488 ↗

(On Diff #30865)

How worried are you about eventual incompatibilities between the flags in these groups for the driver and for the underlying SHAVE compiler? I'm moderately worried...

How many flags are we talking about here? Also, the -W flags worry me less than the rest.

Finally, why two AddAllArgs?

[jyknight]

lib/Driver/Tools.cpp

9482–9488 ↗

(On Diff #30865)

How worried are you about eventual incompatibilities between the flags in these groups for the driver and for the underlying SHAVE compiler? I'm moderately worried...

Well, what bad would happen? The thing I'd expect is to have an arg that's been removed from the driver but is still supported by the underlying compiler, or, is not yet present in the underlying compiler but is supported in the driver. Neither of those seem terribly problematic -- you'll get an error from one layer or the other, which is basically the correct thing to have happen.

I think the only actual bad case is if a given flag changes between having a separate value to not, or vice versa, and that seems unlikely enough to come up that it can be ignored as a problem.

why two AddAllArgs?

Because AddAllArgs can take 3 args, and only 3 args, and that's true all the way down through "class arg_iterator", which has 3 members, "OptSpecifier Id0, Id1, Id2", rather than, say, a SmallVector<OptSpecifier, 3> Ids. Yuk.

[dougk]

Make fewer calls to AddAllArgs.

[jyknight]

lgtm

cfe/trunk/lib/Driver/Tools.cpp

void tools::SHAVE::Compiler::ConstructJob(Compilation &C, const JobAction &JA,
 
   ArgStringList CmdArgs;
 
   assert(Inputs.size() == 1);
   const InputInfo &II = Inputs[0];
   assert(II.getType() == types::TY_C || II.getType() == types::TY_CXX);
   assert(Output.getType() == types::TY_PP_Asm); // Require preprocessed asm.
 
-  // Append all -I, -iquote, -isystem paths, and defines/undefines.
-  // These are spelled the same way in clang and moviCompile.
-  Args.AddAllArgs(CmdArgs,
-                  {options::OPT_clang_i_Group, options::OPT_D, options::OPT_U});
-
   CmdArgs.push_back("-DMYRIAD2");
   CmdArgs.push_back("-mcpu=myriad2");
   CmdArgs.push_back("-S");
 
-  // Any -O option passes through without translation. What about -Ofast ?
-  if (Arg *A = Args.getLastArg(options::OPT_O_Group))
-    A->render(Args, CmdArgs);
-
-  if (Args.hasFlag(options::OPT_ffunction_sections,
-                   options::OPT_fno_function_sections)) {
-    CmdArgs.push_back("-ffunction-sections");
-  }
-  if (Args.hasArg(options::OPT_fno_inline_functions))
-    CmdArgs.push_back("-fno-inline-functions");
-
+  // Append all -I, -iquote, -isystem paths, defines/undefines,
+  // 'f' flags, optimize flags, and warning options.
+  // These are spelled the same way in clang and moviCompile.
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_I_Group, options::OPT_clang_i_Group,
+                   options::OPT_D, options::OPT_U,
+                   options::OPT_f_Group,
+                   options::OPT_f_clang_Group,
+                   options::OPT_g_Group,
+                   options::OPT_O_Group,
+                   options::OPT_W_Group});
   CmdArgs.push_back("-fno-exceptions"); // Always do this even if unspecified.
 
   CmdArgs.push_back(II.getFilename());
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
   std::string Exec =
       Args.MakeArgString(getToolChain().GetProgramPath("moviCompile"));

cfe/trunk/test/Driver/shave-toolchain.c

 // Ensure that '-target shave' picks a different compiler.
 // Also check that '-I' is turned into '-i:' for the assembler.
 
 // Note that since we don't know where movi tools are installed,
 // the driver may or may not find a full path to them.
 // That is, the 0th argument will be "/path/to/my/moviCompile"
 // or just "moviCompile" depending on whether moviCompile is found.
 // As such, we test only for a trailing quote in its rendering.
 // The same goes for "moviAsm".
 
 // RUN: %clang -target shave -c -### %s -Icommon 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=movicompile
-// movicompile: moviCompile" "-DMYRIAD2"
+// movicompile: moviCompile" "-DMYRIAD2" "-mcpu=myriad2" "-S" "-I" "common"
 // movicompile: moviAsm" "-no6thSlotCompression" "-cv:myriad2" "-noSPrefixing" "-a" "-i:common" "-elf"
 
 // RUN: %clang -target shave -c -### %s -DEFINE_ME -UNDEFINE_ME 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=defines
 // defines: "-D" "EFINE_ME" "-U" "NDEFINE_ME"
 
 // RUN: %clang -target shave -c -### %s -Icommon -iquote quotepath -isystem syspath 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=includes
 // includes: "-iquote" "quotepath" "-isystem" "syspath"
+
+// RUN: %clang -target shave -c -### %s -g -fno-inline-functions \
+// RUN: -fno-inline-functions-called-once -Os -Wall \
+// RUN: -ffunction-sections 2>&1 | FileCheck %s -check-prefix=F_G_O_W_OPTIONS
+// F_G_O_W_OPTIONS: "-g" "-fno-inline-functions" "-fno-inline-functions-called-once"
+// F_G_O_W_OPTIONS: "-Os" "-Wall" "-ffunction-sections"