[Driver] Add support for -finline-functions and /Ob2 flags

[Ilod]

Summary

-finline-functions and /Ob2 are currently ignored by Clang. The only way to enable inlining is to use the global O flags, which also enable other options, or to emit LLVM bitcode using Clang, then running opt by hand with the inline pass.
This patch allows to simply use the -finline-functions flag (same as GCC) or /Ob2 in clang-cl mode to enable inlining without other optimizations.
This is the first patch of a serie to improve support for the /Ob flags.

[hans]

Thanks! Some comments below:

lib/Driver/Tools.cpp

5335–5337

Please line break this; lines should be <= 80 columns wide.

5337

I think this can be:

InlineArg->render(Args, CmdArgs)

instead

lib/Frontend/CompilerInvocation.cpp

446

This line and 444 need line breaks too.

test/CodeGen/inline-optim.c

17

I'd suggest using "check-label" for the @foo checks. See http://llvm.org/docs/CommandGuide/FileCheck.html#the-check-label-directive

25

For all these checks, I'd suggest matching e.g.

"call i32 @inline_hint"

to make it a little easier to see what' being matched for. I suppose your checks could match either the calls, or the definitions of the called function.

[Ilod]

Thanks!

Updated to respect the 80-columns limit.
Updated the tests to have a clearer checks. (I don't know well the FileCheck, so I took example from test/CodeGen/noinline.c, but this is indeed more comprehensible).

[hans]

Looks good to me!

Do you have commit access, or would you like me to commit this for you?

[Ilod]

I don't have commit access, so you can do it for me, thanks.

[hans]

majnemer reminded me that /O options can be combined, so e.g. /Odb2 would be a valid combination. I've followed up with r270614 to move the /Ob flags into that mechanism.

[Ilod]

Hello,

It seems the new test was commited as .cc instead of .c, which changes the name mangling, making // NOINLINE-LABEL: @foo fail.
Either the file should be renamed, or -x c should be added to clang invocation, or LABEL check updated.

[hans]

In D20576#438581, @Ilod wrote:

It seems the new test was commited as .cc instead of .c, which changes the name mangling, making // NOINLINE-LABEL: @foo fail.
Either the file should be renamed, or -x c should be added to clang invocation, or LABEL check updated.

Sigh, I'm not getting many points today.

I renamed the test because when I tried it, the inline_hint() function didn't get inlined in C mode, but it did in C++ mode. But in so doing, I renamed it to .cc, which means the test didn't run at all (it should be .cpp), which is why I didn't notice that the test doesn't pass.

The reason inlining didn't happen was probably because I ran the test for a different target than you did. Which reminds me, the test needs to specify a target triple.

Hopefully I managed to make things right in r270633

include/clang/Driver/CLCompatOptions.td

def _SLASH_I : CLJoinedOrSeparate<"I">,
   Alias<I>;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
   Alias<funsigned_char>;
 def _SLASH_O0 : CLFlag<"O0">, Alias<O0>;
 // /Oy- is handled by the /O option because /Oy- only has an effect on 32-bit.
 def _SLASH_O : CLJoined<"O">, HelpText<"Optimization level">;
 def _SLASH_Ob0 : CLFlag<"Ob0">, HelpText<"Disable inlining">,
   Alias<fno_inline>;
+def _SLASH_Ob2 : CLFlag<"Ob2">, HelpText<"Enable inlining">,
+  Alias<finline_functions>;
 def _SLASH_Od : CLFlag<"Od">, HelpText<"Disable optimization">, Alias<O0>;
 def _SLASH_Oi : CLFlag<"Oi">, HelpText<"Enable use of builtin functions">,
   Alias<fbuiltin>;
 def _SLASH_Oi_ : CLFlag<"Oi-">, HelpText<"Disable use of builtin functions">,
   Alias<fno_builtin>;
 def _SLASH_Os : CLFlag<"Os">, HelpText<"Optimize for size">, Alias<O>,
   AliasArgs<["s"]>;
 def _SLASH_Ot : CLFlag<"Ot">, HelpText<"Optimize for speed">, Alias<O>,

 def _SLASH_Fd : CLIgnoredJoined<"Fd">;
 def _SLASH_FC : CLIgnoredFlag<"FC">;
 def _SLASH_FS : CLIgnoredFlag<"FS">, HelpText<"Force synchronous PDB writes">;
 def _SLASH_GF : CLIgnoredFlag<"GF">;
 def _SLASH_GS_ : CLIgnoredFlag<"GS-">;
 def _SLASH_kernel_ : CLIgnoredFlag<"kernel-">;
 def _SLASH_nologo : CLIgnoredFlag<"nologo">;
 def _SLASH_Ob1 : CLIgnoredFlag<"Ob1">;
-def _SLASH_Ob2 : CLIgnoredFlag<"Ob2">;
 def _SLASH_Og : CLIgnoredFlag<"Og">;
 def _SLASH_openmp_ : CLIgnoredFlag<"openmp-">;
 def _SLASH_RTC : CLIgnoredJoined<"RTC">;
 def _SLASH_sdl : CLIgnoredFlag<"sdl">;
 def _SLASH_sdl_ : CLIgnoredFlag<"sdl-">;
 def _SLASH_w : CLIgnoredJoined<"w">;
 def _SLASH_Zc_auto : CLIgnoredFlag<"Zc:auto">;
 def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;

include/clang/Driver/Options.td

 def fgnu89_inline : Flag<["-"], "fgnu89-inline">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Use the gnu89 inline semantics">;
 def fno_gnu89_inline : Flag<["-"], "fno-gnu89-inline">, Group<f_Group>;
 def fgnu_runtime : Flag<["-"], "fgnu-runtime">, Group<f_Group>,
   HelpText<"Generate output compatible with the standard GNU Objective-C runtime">;
 def fheinous_gnu_extensions : Flag<["-"], "fheinous-gnu-extensions">, Flags<[CC1Option]>;
 def filelist : Separate<["-"], "filelist">, Flags<[LinkerInput]>;
 def : Flag<["-"], "findirect-virtual-calls">, Alias<fapple_kext>;
-def finline_functions : Flag<["-"], "finline-functions">, Group<clang_ignored_gcc_optimization_f_Group>;
+def finline_functions : Flag<["-"], "finline-functions">, Group<f_clang_Group>, Flags<[CC1Option]>;
 def finline : Flag<["-"], "finline">, Group<clang_ignored_f_Group>;
 def finput_charset_EQ : Joined<["-"], "finput-charset=">, Group<f_Group>;
 def fexec_charset_EQ : Joined<["-"], "fexec-charset=">, Group<f_Group>;
 def finstrument_functions : Flag<["-"], "finstrument-functions">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Generate calls to instrument function entry and exit">;
 def flat__namespace : Flag<["-"], "flat_namespace">;
 def flax_vector_conversions : Flag<["-"], "flax-vector-conversions">, Group<f_Group>;
 def flimited_precision_EQ : Joined<["-"], "flimited-precision=">, Group<f_Group>;

lib/Driver/Tools.cpp

 
   if (Args.hasFlag(options::OPT_fgnu89_inline, options::OPT_fno_gnu89_inline,
                    false))
     CmdArgs.push_back("-fgnu89-inline");
 
   if (Args.hasArg(options::OPT_fno_inline))
     CmdArgs.push_back("-fno-inline");
 
-  if (Args.hasArg(options::OPT_fno_inline_functions))
-    CmdArgs.push_back("-fno-inline-functions");
+  if (Arg* InlineArg = Args.getLastArg(options::OPT_finline_functions,
+                                       options::OPT_fno_inline_functions))
+    InlineArg->render(Args, CmdArgs);
 
   ObjCRuntime objcRuntime = AddObjCRuntimeArgs(Args, CmdArgs, rewriteKind);
 
   // -fobjc-dispatch-method is only relevant with the nonfragile-abi, and
   // legacy is the default. Except for deployment taget of 10.5,
   // next runtime is always legacy dispatch and -fno-objc-legacy-dispatch
   // gets ignored silently.
   if (objcRuntime.isNonFragile()) {

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.OptimizationLevel = OptimizationLevel;
 
   // We must always run at least the always inlining pass.
   Opts.setInlining(
     (Opts.OptimizationLevel > 1) ? CodeGenOptions::NormalInlining
                                  : CodeGenOptions::OnlyAlwaysInlining);
   // -fno-inline-functions overrides OptimizationLevel > 1.
   Opts.NoInline = Args.hasArg(OPT_fno_inline);
-  Opts.setInlining(Args.hasArg(OPT_fno_inline_functions) ?
-                     CodeGenOptions::OnlyAlwaysInlining : Opts.getInlining());
+  if (Arg* InlineArg = Args.getLastArg(options::OPT_finline_functions,
+                                       options::OPT_fno_inline_functions)) {
+    Opts.setInlining(
+      InlineArg->getOption().matches(options::OPT_finline_functions) ?
+        CodeGenOptions::NormalInlining : CodeGenOptions::OnlyAlwaysInlining);
+  }
 
   if (Arg *A = Args.getLastArg(OPT_fveclib)) {
     StringRef Name = A->getValue();
     if (Name == "Accelerate")
       Opts.setVecLib(CodeGenOptions::Accelerate);
     else if (Name == "none")
       Opts.setVecLib(CodeGenOptions::NoLibrary);
     else

test/CodeGen/inline-optim.c

+// Make sure -finline-functions family flags are behaving correctly.
+
+// RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck -check-prefix=NOINLINE %s
+// RUN: %clang_cc1 -O3 -fno-inline-functions -emit-llvm %s -o - | FileCheck -check-prefix=NOINLINE %s
+// RUN: %clang_cc1 -finline-functions -emit-llvm %s -o - | FileCheck -check-prefix=INLINE %s
+
+inline int inline_hint(int a, int b) { return(a+b); }
+
+int inline_no_hint(int a, int b) { return (a/b); }
+
+inline __attribute__ ((__always_inline__)) int inline_always(int a, int b) { return(a*b); }
+
+volatile int *pa = (int*) 0x1000;
+void foo() {
+// NOINLINE-LABEL: @foo
+// INLINE-LABEL: @foo
+// NOINLINE: call i32 @inline_hint
+// INLINE-NOT: call i32 @inline_hint
+    pa[0] = inline_hint(pa[1],pa[2]);  
+// NOINLINE-NOT: call i32 @inline_always
+// INLINE-NOT: call i32 @inline_always
+    pa[3] = inline_always(pa[4],pa[5]);
+// NOINLINE: call i32 @inline_no_hint
+// INLINE-NOT: call i32 @inline_no_hint
+    pa[6] = inline_no_hint(pa[7], pa[8]);
+}

test/Driver/cl-options.c

 // J: -fno-signed-char
 
 // RUN: %clang_cl /Ofoo -### -- %s 2>&1 | FileCheck -check-prefix=O %s
 // O: /Ofoo
 
 // RUN: %clang_cl /Ob0 -### -- %s 2>&1 | FileCheck -check-prefix=Ob0 %s
 // Ob0: -fno-inline
 
+// RUN: %clang_cl /Ob2 -### -- %s 2>&1 | FileCheck -check-prefix=Ob2 %s
+// Ob2: -finline-functions
+
 // RUN: %clang_cl /Od -### -- %s 2>&1 | FileCheck -check-prefix=Od %s
 // Od: -O0
 
 // RUN: %clang_cl /Oi- /Oi -### -- %s 2>&1 | FileCheck -check-prefix=Oi %s
 // Oi-NOT: -fno-builtin
 
 // RUN: %clang_cl /Oi- -### -- %s 2>&1 | FileCheck -check-prefix=Oi_ %s
 // Oi_: -fno-builtin

 // RUN:    /Fdfoo \
 // RUN:    /FS \
 // RUN:    /Gd \
 // RUN:    /GF \
 // RUN:    /GS- \
 // RUN:    /kernel- \
 // RUN:    /nologo \
 // RUN:    /Ob1 \
-// RUN:    /Ob2 \
 // RUN:    /openmp- \
 // RUN:    /RTC1 \
 // RUN:    /sdl \
 // RUN:    /sdl- \
 // RUN:    /vmg \
 // RUN:    /volatile:iso \
 // RUN:    /w12345 \
 // RUN:    /wd1234 \

test/Driver/clang_f_opts.c

 
 // Test that the warning is displayed on these.
 // RUN: %clang -###                                                           \
 // RUN: -finline-limit=1000                                                   \
 // RUN: -finline-limit                                                        \
 // RUN: -fexpensive-optimizations                                             \
 // RUN: -fno-expensive-optimizations                                          \
 // RUN: -fno-defer-pop                                                        \
-// RUN: -finline-functions                                                    \
 // RUN: -fkeep-inline-functions                                               \
 // RUN: -fno-keep-inline-functions                                            \
 // RUN: -freorder-blocks                                                      \
 // RUN: -fprofile-dir=/rand/dir                                               \
 // RUN: -falign-functions                                                     \
 // RUN: -falign-functions=1                                                   \
 // RUN: -ffloat-store                                                         \
 // RUN: -fgcse                                                                \

 // RUN: -fno-devirtualize                                                     \
 // RUN: -fno-devirtualize-speculatively                                       \
 // RUN: %s 2>&1 | FileCheck --check-prefix=CHECK-WARNING %s
 // CHECK-WARNING-DAG: optimization flag '-finline-limit=1000' is not supported
 // CHECK-WARNING-DAG: optimization flag '-finline-limit' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fexpensive-optimizations' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fno-expensive-optimizations' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fno-defer-pop' is not supported
-// CHECK-WARNING-DAG: optimization flag '-finline-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fkeep-inline-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fno-keep-inline-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-freorder-blocks' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fprofile-dir=/rand/dir' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-functions' is not supported
 // CHECK-WARNING-DAG: optimization flag '-falign-functions=1' is not supported
 // CHECK-WARNING-DAG: optimization flag '-ffloat-store' is not supported
 // CHECK-WARNING-DAG: optimization flag '-fgcse' is not supported