Add fveclib option and add several entries to table of vectorizable functions.

[mzolotukhin]

Summary

Add fveclib option.

Partially fill the list of vectorizable functions for framework
Accelerate.

This is the last, sixth, of 6 patches for enabling vectorization of calls.

[hfinkel]

lib/CodeGen/BackendUtil.cpp

253

I think that I'd still rather add the functions to TLI inside of TLI. I like the idea that the frontend can add others, and we should keep that capability. However, there is no reason that other frontends should not be able to take advantage of this knowledge. We should translate the Clang-level CodeGenOpt into LLVM-level CodeGenOpts, and then add these there.

lib/Driver/Tools.cpp

2854

You don't need this if.

Furthermore, I think that this whole thing can be replaced by:

Args.AddLastArg(CmdArgs, options::OPT_fveclib);

[mzolotukhin]

Address Hal's remarks:
- Simplify arg passing.
- Move initialization of the vectorizable-function tables into TLI.

[mzolotukhin]

Hi Hal,
I moved the tables initialization to TLI, the corresponding TLI patch is here:
http://reviews.llvm.org/D8131
Now frontends can either use addVectorizableFunctionsFromVecLib to initialize the tables for a known library, or manually fill the table and use addVectorizableFunctions. Did I get it right, that it's what you wanted to keep?

[hfinkel]

include/clang/Frontend/CodeGenOptions.def

164

All of the entries here have a descriptive comment above them, please add one here as well.

include/clang/Frontend/CodeGenOptions.h

51

Use -> Use the

lib/Frontend/CompilerInvocation.cpp

375

I'd rather we actually validated the value here, instead of just mapping all unknown strings to 'NoLibrary'. I'd rather something like this:

if (Arg *A = Args.getLastArg(OPT_fveclib)) {
  if (A->getValue() == "Accelerate")
    Opts.setVecLib(CodeGenOptions::Accelerate);
  else if (A->getValue() == "none")
    Opts.setVecLib(CodeGenOptions::NoLibrary);
  else
    Diags.Report(diag::err_drv_invalid_value)
      << A->getAsString(Args) << A->getValue();
}

[hfinkel]

• Original Message -----

From: "Michael Zolotukhin" <[e-mail]>
To: [e-mail], [e-mail], [e-mail], [e-mail], "james molloy"
<[e-mail]>
Cc: [e-mail]
Sent: Friday, March 6, 2015 7:25:52 PM
Subject: Re: [PATCH] Add fveclib option and add several entries to table of vectorizable functions.

Hi Hal,
I moved the tables initialization to TLI, the corresponding TLI patch
is here:
http://reviews.llvm.org/D8131
Now frontends can either use addVectorizableFunctionsFromVecLib to
initialize the tables for a known library, or manually fill the
table and use addVectorizableFunctions. Did I get it right, that
it's what you wanted to keep?

I think this makes sense.

-Hal

http://reviews.llvm.org/D8097

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

[mzolotukhin]

• Report unrecognized value in fveclib CL option.

[hfinkel]

This LGTM, but needs a regression test for the command-line handling before it can be committed (that we accept -fveclib=Accelerate and -fveclib=none, that the last one wins, and that we get an error on an unknown one). There are lots of potential examples, see test/Driver/code-model.c, test/Driver/visibility.cpp.

include/clang/Driver/Options.td

761

Remove blank line.

[mzolotukhin]

• Add regression tests for '-fveclib' option.

[mzolotukhin]

• Remove blank line.

[hfinkel]

As per our IRC discussion, setting the linking flags for the Accelerate library when provided -fveclib=Accelerate will be added in follow-up. This LGTM.

[mzolotukhin]

Thanks, committed in r232533.

include/clang/Driver/Options.td

def fno_dollars_in_identifiers : Flag<["-"], "fno-dollars-in-identifiers">, Group<f_Group>,
   HelpText<"Disallow '$' in identifiers">, Flags<[CC1Option]>;
 def fno_elide_constructors : Flag<["-"], "fno-elide-constructors">, Group<f_Group>,
   HelpText<"Disable C++ copy constructor elision">, Flags<[CC1Option]>;
 def fno_eliminate_unused_debug_symbols : Flag<["-"], "fno-eliminate-unused-debug-symbols">, Group<f_Group>;
 def fno_exceptions : Flag<["-"], "fno-exceptions">, Group<f_Group>;
 def fno_gnu_keywords : Flag<["-"], "fno-gnu-keywords">, Group<f_Group>, Flags<[CC1Option]>;
 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, Group<f_clang_Group>, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group<f_clang_Group>, Flags<[CC1Option]>;
+def fveclib : Joined<["-"], "fveclib=">, Group<f_Group>, Flags<[CC1Option]>,
+    HelpText<"Use the given vector functions library">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, Group<f_Group>,
   HelpText<"Disallow implicit conversions between vectors with a different number of elements or different element types">, Flags<[CC1Option]>;
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group<f_Group>,
     Flags<[CC1Option]>, HelpText<"Disallow merging of constants">;
 def fno_modules : Flag <["-"], "fno-modules">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fno_module_maps : Flag <["-"], "fno-module-maps">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fno_modules_decluse : Flag <["-"], "fno-modules-decluse">, Group<f_Group>,

include/clang/Frontend/CodeGenOptions.h

 class CodeGenOptions : public CodeGenOptionsBase {
 public:
   enum InliningMethod {
     NoInlining,         // Perform no inlining whatsoever.
     NormalInlining,     // Use the standard function inlining pass.
     OnlyAlwaysInlining  // Only run the always inlining pass.
   };
 
+  enum VectorLibrary {
+    NoLibrary, // Don't use any vector library.
+    Accelerate // Use the Accelerate framework.
+  };
+
   enum ObjCDispatchMethodKind {
     Legacy = 0,
     NonLegacy = 1,
     Mixed = 2
   };
 
   enum DebugInfoKind {
     NoDebugInfo,          /// Don't generate debug info.

include/clang/Frontend/CodeGenOptions.def

 ENUM_CODEGENOPT(DebugInfo, DebugInfoKind, 3, NoDebugInfo)
 
 /// Dwarf version.
 VALUE_CODEGENOPT(DwarfVersion, 3, 0)
 
 /// The kind of inlining to perform.
 ENUM_CODEGENOPT(Inlining, InliningMethod, 2, NoInlining)
 
+// Vector functions library to use.
+ENUM_CODEGENOPT(VecLib, VectorLibrary, 1, NoLibrary)
+
 /// The default TLS model to use.
 ENUM_CODEGENOPT(DefaultTLSModel, TLSModel, 2, GeneralDynamicTLSModel)
 
 #undef CODEGENOPT
 #undef ENUM_CODEGENOPT
 #undef VALUE_CODEGENOPT
 

lib/CodeGen/BackendUtil.cpp

static void addDataFlowSanitizerPass(const PassManagerBuilder &Builder,
   PM.add(createDataFlowSanitizerPass(LangOpts.SanitizerBlacklistFiles));
 }
 
 static TargetLibraryInfoImpl *createTLII(llvm::Triple &TargetTriple,
                                          const CodeGenOptions &CodeGenOpts) {
   TargetLibraryInfoImpl *TLII = new TargetLibraryInfoImpl(TargetTriple);
   if (!CodeGenOpts.SimplifyLibCalls)
     TLII->disableAllFunctions();
+
+  switch (CodeGenOpts.getVecLib()) {
+  case CodeGenOptions::Accelerate:
+    TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::Accelerate);
+    break;
+  default:
+    break;
+  }
   return TLII;
 }
 
 static void addSymbolRewriterPass(const CodeGenOptions &Opts,
                                   legacy::PassManager *MPM) {
   llvm::SymbolRewriter::RewriteDescriptorList DL;
 
   llvm::SymbolRewriter::RewriteMapParser MapParser;
   for (const auto &MapFile : Opts.RewriteMapFiles)
     MapParser.parse(MapFile, &DL);
 
   MPM->add(createRewriteSymbolsPass(DL));
 }
 

lib/Driver/Tools.cpp

#endif
   }
 
   CmdArgs.push_back("-mthread-model");
   if (Arg *A = Args.getLastArg(options::OPT_mthread_model))
     CmdArgs.push_back(A->getValue());
   else
     CmdArgs.push_back(Args.MakeArgString(getToolChain().getThreadModel()));
 
+  Args.AddLastArg(CmdArgs, options::OPT_fveclib);
+
   if (!Args.hasFlag(options::OPT_fmerge_all_constants,
                     options::OPT_fno_merge_all_constants))
     CmdArgs.push_back("-fno-merge-all-constants");
 
   // LLVM Code Generator Options.
 
   if (Args.hasArg(options::OPT_frewrite_map_file) ||
       Args.hasArg(options::OPT_frewrite_map_file_EQ)) {

lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   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 *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
+      Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
+  }
+
   if (Args.hasArg(OPT_gline_tables_only)) {
     Opts.setDebugInfo(CodeGenOptions::DebugLineTablesOnly);
   } else if (Args.hasArg(OPT_g_Flag) || Args.hasArg(OPT_gdwarf_2) ||
              Args.hasArg(OPT_gdwarf_3) || Args.hasArg(OPT_gdwarf_4)) {
     bool Default = false;
     // Until dtrace (via CTF) and LLDB can deal with distributed debug info,
     // Darwin and FreeBSD default to standalone/full debug info.
     if (llvm::Triple(TargetOpts.Triple).isOSDarwin() ||

test/Driver/fveclib.c

+// RUN: %clang -### -c -fveclib=none %s 2>&1 | FileCheck -check-prefix CHECK-NOLIB %s
+// RUN: %clang -### -c -fveclib=Accelerate %s 2>&1 | FileCheck -check-prefix CHECK-ACCELERATE %s
+// RUN: not %clang -c -fveclib=something %s 2>&1 | FileCheck -check-prefix CHECK-INVALID %s
+
+// CHECK-NOLIB: "-fveclib=none"
+// CHECK-ACCELERATE: "-fveclib=Accelerate"
+
+// CHECK-INVALID: error: invalid value 'something' in '-fveclib=something'