Attach attribute "trap-func-name" to IR function

[ahatanak]

Summary

This patch is part of the work to make LTO and function multi-versioning work correctly.

Currently, the trap function name users specify on the command line using -ftrap-function is passed to the backend via TargetMachine's TargetOptions. This patch attaches attribute "trap-func-name" to the IR function instead so that we can use -ftrap-function for LTO builds and change the trap function name on a per-function basis.

[echristo]

Not sure we necessarily need a "put these function attributes on the functions" command line and map? Rationale?

-eric

[ahatanak]

In D10831#196951, @echristo wrote:

Not sure we necessarily need a "put these function attributes on the functions" command line and map? Rationale?

That's a good point. I don't think we need a new cc1 option for trap-function at all.

[ahatanak]

Remove cc1 option and simplify patch.

[echristo]

Lgtm thanks.

[ahatanak]

Thanks, I'll send the corresponding llvm patch shortly.

[ahatanak]

The new patch attaches "trap-func-name" to the call sites instead of the calling functions, which simplifies the handling of the attribute when functions are merged and inlined.

The relevant discussion I had with Eric is here:
http://reviews.llvm.org/D10832

[echristo]

You'll want to also test a trap inside of a ctor/dtor, global constructor (if you can), etc. The general change looks fine though.

Thanks!

-eric

[ahatanak]

I wasn't sure how to test traps inside ctor/dtor or global constructor, so I went ahead and committed the reviewed patch. I can add the tests later if I can find a way to emit traps for those functions.

[pcc]

Maybe I'm missing something, but I don't see why we need this attribute. Couldn't clang have been changed to implement -ftrap-function by generating a call to the trap function instead of emitting an llvm.trap intrinsic call?

[ahatanak]

AMDGPUAnnotateKernelFeatures::addFeatureAttributes looks for an llvm.trap instruction and adds attribute "amdgpu-queue-ptr" to the function if it finds one.

Other than that, it looks like your idea would work too.

include/clang/Driver/CC1Options.td

 def mlimit_float_precision : Separate<["-"], "mlimit-float-precision">,
   HelpText<"Limit float precision to the given value">;
 def split_stacks : Flag<["-"], "split-stacks">,
   HelpText<"Try to use a split stack if possible.">;
 def mno_zero_initialized_in_bss : Flag<["-"], "mno-zero-initialized-in-bss">,
   HelpText<"Do not put zero initialized data in the BSS">;
 def backend_option : Separate<["-"], "backend-option">,
   HelpText<"Additional arguments to forward to LLVM backend (during code gen)">;
+def function_attribute : Separate<["-"], "function-attribute">,
+  HelpText<"Function attributes to attach to functions (during code gen)">;
 def mregparm : Separate<["-"], "mregparm">,
   HelpText<"Limit the number of registers available for integer arguments">;
 def mrelocation_model : Separate<["-"], "mrelocation-model">,
   HelpText<"The relocation model to use">;
 def munwind_tables : Flag<["-"], "munwind-tables">,
   HelpText<"Generate unwinding tables for all functions">;
 def mconstructor_aliases : Flag<["-"], "mconstructor-aliases">,
   HelpText<"Emit complete constructors and destructors as aliases when possible">;

include/clang/Frontend/CodeGenOptions.h

public:
 
   /// If not an empty string, trap intrinsics are lowered to calls to this
   /// function instead of to trap instructions.
   std::string TrapFuncName;
 
   /// A list of command-line options to forward to the LLVM backend.
   std::vector<std::string> BackendOptions;
 
+  /// A list of function attributes in the form of key-value pairs to save to
+  /// the IR.
+  std::vector<std::pair<std::string, std::string>> FunctionAttributes;
+
   /// A list of dependent libraries.
   std::vector<std::string> DependentLibraries;
 
   /// Name of the profile file to use as output for -fprofile-instr-generate
   std::string InstrProfileOutput;
 
   /// Name of the profile file to use with -fprofile-sample-use.
   std::string SampleProfileFile;

lib/CodeGen/BackendUtil.cpp

TargetMachine *EmitAssemblyHelper::CreateTargetMachine(bool MustCreateTM) {
   }
 
   Options.LessPreciseFPMADOption = CodeGenOpts.LessPreciseFPMAD;
   Options.NoInfsFPMath = CodeGenOpts.NoInfsFPMath;
   Options.NoNaNsFPMath = CodeGenOpts.NoNaNsFPMath;
   Options.NoZerosInBSS = CodeGenOpts.NoZeroInitializedInBSS;
   Options.UnsafeFPMath = CodeGenOpts.UnsafeFPMath;
   Options.StackAlignmentOverride = CodeGenOpts.StackAlignment;
-  Options.TrapFuncName = CodeGenOpts.TrapFuncName;
   Options.PositionIndependentExecutable = LangOpts.PIELevel != 0;
   Options.FunctionSections = CodeGenOpts.FunctionSections;
   Options.DataSections = CodeGenOpts.DataSections;
   Options.UniqueSectionNames = CodeGenOpts.UniqueSectionNames;
 
   Options.MCOptions.MCRelaxAll = CodeGenOpts.RelaxAll;
   Options.MCOptions.MCSaveTempLabels = CodeGenOpts.SaveTempLabels;
   Options.MCOptions.MCUseDwarfDirectory = !CodeGenOpts.NoDwarfDirectoryAsm;

lib/CodeGen/CGCall.cpp

if (CodeGenOpts.EnableSegmentedStacks &&
     FuncAttrs.addAttribute("split-stack");
 
   if (AttrOnCallSite) {
     // Attributes that should go on the call site only.
     if (!CodeGenOpts.SimplifyLibCalls)
       FuncAttrs.addAttribute(llvm::Attribute::NoBuiltin);
   } else {
     // Attributes that should go on the function, but not the call site.
+    for (auto &KV : CodeGenOpts.FunctionAttributes)
+      FuncAttrs.addAttribute(KV.first, KV.second);
+
     if (!CodeGenOpts.DisableFPElim) {
       FuncAttrs.addAttribute("no-frame-pointer-elim", "false");
     } else if (CodeGenOpts.OmitLeafFramePointer) {
       FuncAttrs.addAttribute("no-frame-pointer-elim", "false");
       FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");
     } else {
       FuncAttrs.addAttribute("no-frame-pointer-elim", "true");
       FuncAttrs.addAttribute("no-frame-pointer-elim-non-leaf");

lib/Driver/Tools.cpp

   Args.AddLastArg(CmdArgs, options::OPT_ftime_report);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
 
   if (Arg *A = Args.getLastArg(options::OPT_ftrapv_handler_EQ)) {
     CmdArgs.push_back("-ftrapv-handler");
     CmdArgs.push_back(A->getValue());
   }
 
-  Args.AddLastArg(CmdArgs, options::OPT_ftrap_function_EQ);
+  if (Arg *A = Args.getLastArg(options::OPT_ftrap_function_EQ)) {
+    CmdArgs.push_back("-function-attribute");
+    auto AttrStr = Twine("trap-func-name=") + A->getValue();
+    CmdArgs.push_back(Args.MakeArgString(AttrStr));
+  }
 
   // -fno-strict-overflow implies -fwrapv if it isn't disabled, but
   // -fstrict-overflow won't turn off an explicitly enabled -fwrapv.
   if (Arg *A = Args.getLastArg(options::OPT_fwrapv, options::OPT_fno_wrapv)) {
     if (A->getOption().matches(options::OPT_fwrapv))
       CmdArgs.push_back("-fwrapv");
   } else if (Arg *A = Args.getLastArg(options::OPT_fstrict_overflow,
                                       options::OPT_fno_strict_overflow)) {

lib/Frontend/CompilerInvocation.cpp

for (const auto &Sanitizer : Sanitizers) {
     SanitizerMask K = parseSanitizerValue(Sanitizer, /*AllowGroups=*/false);
     if (K == 0)
       Diags.Report(diag::err_drv_invalid_value) << FlagName << Sanitizer;
     else
       S.set(K, true);
   }
 }
 
+static void getFunctionAttributes(CodeGenOptions &Opts,
+                                  const std::vector<std::string> &FnAttrStrs) {
+  for (const auto &S : FnAttrStrs) {
+    std::pair<StringRef, StringRef> KV = StringRef(S).split('=');
+    Opts.FunctionAttributes.push_back(std::make_pair(KV.first, KV.second));
+  }
+}
+
 static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
                              DiagnosticsEngine &Diags,
                              const TargetOptions &TargetOpts) {
   using namespace options;
   bool Success = true;
 
   unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
   // TODO: This could be done in Driver
Opts.UnsafeFPMath = Args.hasArg(OPT_menable_unsafe_fp_math) ||
                       Args.hasArg(OPT_cl_fast_relaxed_math);
   Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
   Opts.RelocationModel = Args.getLastArgValue(OPT_mrelocation_model, "pic");
   Opts.ThreadModel = Args.getLastArgValue(OPT_mthread_model, "posix");
   if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
     Diags.Report(diag::err_drv_invalid_value)
         << Args.getLastArg(OPT_mthread_model)->getAsString(Args)
         << Opts.ThreadModel;
-  Opts.TrapFuncName = Args.getLastArgValue(OPT_ftrap_function_EQ);
   Opts.UseInitArray = Args.hasArg(OPT_fuse_init_array);
 
   Opts.FunctionSections = Args.hasFlag(OPT_ffunction_sections,
                                        OPT_fno_function_sections, false);
   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);
static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   // FIXME: Report unrecoverable sanitizers incorrectly specified here.
   parseSanitizerKinds("-fsanitize-recover=",
                       Args.getAllArgValues(OPT_fsanitize_recover_EQ), Diags,
                       Opts.SanitizeRecover);
   parseSanitizerKinds("-fsanitize-trap=",
                       Args.getAllArgValues(OPT_fsanitize_trap_EQ), Diags,
                       Opts.SanitizeTrap);
 
+  getFunctionAttributes(Opts, Args.getAllArgValues(OPT_function_attribute));
   Opts.CudaGpuBinaryFileNames =
       Args.getAllArgValues(OPT_fcuda_include_gpubinary);
 
   return Success;
 }
 
 static void ParseDependencyOutputArgs(DependencyOutputOptions &Opts,
                                       ArgList &Args) {

test/CodeGen/fn-attr.c

+// RUN: %clang_cc1 -triple thumbv7-apple-ios5 -function-attribute trap-func-name=trapfuncname1 -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: attributes #0 = { {{.*}} "trap-func-name"="trapfuncname1"
+
+int foo1(int a) { return a; }