[Clang FE, SystemZ] Recognize -mpacked-stack CL option

[jonpa]

Summary

Recognize -mpacked-stack from the command line and add a function attribute "mpacked-stack"="true" when passed.

I compared building benchmarks with -mpacked-stack vs master with this change in SystemZFrameLowering.cpp:

static bool usePackedStack(MachineFunction &MF) {
 -  bool HasPackedStackAttr = MF.getFunction().hasFnAttribute("packed-stack");
 +  bool HasPackedStackAttr = true; //MF.getFunction().hasFnAttribute("packed-stack");
    bool IsVarArg = MF.getFunction().isVarArg();

I found that all files look the same, except for all those who contain the __clang_call_terminate function, which gets the default stack layout even when -mpacked-stack is passed. This seems to relate to exceptions, and my guess is this doesn't need to be handled, but it probably could get the "packed-stack" attribute added somewhere in clang/lib/CodeGen if needed...

Would we need to check in the backend that we don't have the attribute "packed-stack"="false", in case somebody would edit that by hand?

[uweigand]

The diag::err_opt_not_valid_on_target message is emitted from CodeGenFunction::StartFunction -- does that mean if you use -mpacked-stack on a non-SystemZ target when compiling a file with many functions, you get this error message many times? If so, this should probably be moved somewhere else. (It looks like the existing check for -mnop-mcount may already have the same problem.

I found that all files look the same, except for all those who contain the __clang_call_terminate function, which gets the default stack layout even when -mpacked-stack is passed. This seems to relate to exceptions, and my guess is this doesn't need to be handled, but it probably could get the "packed-stack" attribute added somewhere in clang/lib/CodeGen if needed...

This function is synthesized in getClangCallTerminateFn and doesn't go through the usual StartFunction. But I guess this doesn't really matter either way.

Would we need to check in the backend that we don't have the attribute "packed-stack"="false", in case somebody would edit that by hand?

Hmm, some places look for an explicit attribute value of "true", e.g. the X86 target here:

bool SoftFloat =
    F.getFnAttribute("use-soft-float").getValueAsString() == "true";

I guess this would be preferable.

[jonpa]

In D71441#1785668, @uweigand wrote:

The diag::err_opt_not_valid_on_target message is emitted from CodeGenFunction::StartFunction -- does that mean if you use -mpacked-stack on a non-SystemZ target when compiling a file with many functions, you get this error message many times? If so, this should probably be moved somewhere else. (It looks like the existing check for -mnop-mcount may already have the same problem.

No, it seems to only print this one time even in a file with multiple functions.

I found that all files look the same, except for all those who contain the __clang_call_terminate function, which gets the default stack layout even when -mpacked-stack is passed. This seems to relate to exceptions, and my guess is this doesn't need to be handled, but it probably could get the "packed-stack" attribute added somewhere in clang/lib/CodeGen if needed...

This function is synthesized in getClangCallTerminateFn and doesn't go through the usual StartFunction. But I guess this doesn't really matter either way.

ok

Would we need to check in the backend that we don't have the attribute "packed-stack"="false", in case somebody would edit that by hand?

Hmm, some places look for an explicit attribute value of "true", e.g. the X86 target here:

bool SoftFloat =
    F.getFnAttribute("use-soft-float").getValueAsString() == "true";

I guess this would be preferable.

OK, I'll make a separate backend-patch for that.

[uweigand]

OK, then I think this patch LGTM.

[jonpa]

Removed the "true" value when adding the function attribute, which seems to do what we want.

[uweigand]

OK, thanks!

[jonpa]

599d1cc

clang/include/clang/Basic/CodeGenOptions.def

 
 ///< Set the minimum number of instructions in a function to determine selective
 ///< XRay instrumentation.
 VALUE_CODEGENOPT(XRayInstructionThreshold , 32, 200)
 
 CODEGENOPT(InstrumentForProfiling , 1, 0) ///< Set when -pg is enabled.
 CODEGENOPT(CallFEntry , 1, 0) ///< Set when -mfentry is enabled.
 CODEGENOPT(MNopMCount , 1, 0) ///< Set when -mnop-mcount is enabled.
+CODEGENOPT(PackedStack , 1, 0) ///< Set when -mpacked-stack is enabled.
 CODEGENOPT(LessPreciseFPMAD  , 1, 0) ///< Enable less precise MAD instructions to
                                      ///< be generated.
 CODEGENOPT(PrepareForLTO     , 1, 0) ///< Set when -flto is enabled on the
                                      ///< compile step.
 CODEGENOPT(PrepareForThinLTO , 1, 0) ///< Set when -flto=thin is enabled on the
                                      ///< compile step.
 CODEGENOPT(LTOUnit, 1, 0) ///< Emit IR to support LTO unit features (CFI, whole
                           ///< program vtable opt).

clang/include/clang/Driver/Options.td

 def mpie_copy_relocations : Flag<["-"], "mpie-copy-relocations">, Group<m_Group>,
   Flags<[CC1Option]>,
   HelpText<"Use copy relocations support for PIE builds">;
 def mno_pie_copy_relocations : Flag<["-"], "mno-pie-copy-relocations">, Group<m_Group>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at function entry (x86/SystemZ only)">,
   Flags<[CC1Option]>, Group<m_Group>;
 def mnop_mcount : Flag<["-"], "mnop-mcount">, HelpText<"Generate mcount/__fentry__ calls as nops. To activate they need to be patched in.">,
   Flags<[CC1Option]>, Group<m_Group>;
+def mpacked_stack : Flag<["-"], "mpacked-stack">, HelpText<"Use packed stack layout (SystemZ only).">,
+  Flags<[CC1Option]>, Group<m_Group>;
 def mips16 : Flag<["-"], "mips16">, Group<m_mips_Features_Group>;
 def mno_mips16 : Flag<["-"], "mno-mips16">, Group<m_mips_Features_Group>;
 def mmicromips : Flag<["-"], "mmicromips">, Group<m_mips_Features_Group>;
 def mno_micromips : Flag<["-"], "mno-micromips">, Group<m_mips_Features_Group>;
 def mxgot : Flag<["-"], "mxgot">, Group<m_mips_Features_Group>;
 def mno_xgot : Flag<["-"], "mno-xgot">, Group<m_mips_Features_Group>;
 def mldc1_sdc1 : Flag<["-"], "mldc1-sdc1">, Group<m_mips_Features_Group>;
 def mno_ldc1_sdc1 : Flag<["-"], "mno-ldc1-sdc1">, Group<m_mips_Features_Group>;

clang/lib/CodeGen/CodeGenFunction.cpp

if (!CurFuncDecl || !CurFuncDecl->hasAttr<NoInstrumentFunctionAttr>()) {
         if (!CGM.getCodeGenOpts().CallFEntry)
           CGM.getDiags().Report(diag::err_opt_not_valid_without_opt)
             << "-mnop-mcount" << "-mfentry";
         Fn->addFnAttr("mnop-mcount", "true");
       }
     }
   }
 
+  if (CGM.getCodeGenOpts().PackedStack) {
+    if (getContext().getTargetInfo().getTriple().getArch() !=
+        llvm::Triple::systemz)
+      CGM.getDiags().Report(diag::err_opt_not_valid_on_target)
+        << "-mpacked-stack";
+    Fn->addFnAttr("packed-stack", "true");
+  }
+
   if (RetTy->isVoidType()) {
     // Void type; nothing to return.
     ReturnValue = Address::invalid();
 
     // Count the implicit return.
     if (!endsWithReturn(D))
       ++NumReturnExprs;
   } else if (CurFnInfo->getReturnInfo().getKind() == ABIArgInfo::Indirect) {

clang/lib/Driver/ToolChains/Clang.cpp

if (TC.SupportsProfiling())
     Args.AddLastArg(CmdArgs, options::OPT_pg);
 
   if (TC.SupportsProfiling())
     Args.AddLastArg(CmdArgs, options::OPT_mfentry);
 
   if (TC.SupportsProfiling())
     Args.AddLastArg(CmdArgs, options::OPT_mnop_mcount);
 
+  Args.AddLastArg(CmdArgs, options::OPT_mpacked_stack);
+
   if (Args.getLastArg(options::OPT_fapple_kext) ||
       (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
     CmdArgs.push_back("-fapple-kext");
 
   Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_print_source_range_info);
   Args.AddLastArg(CmdArgs, options::OPT_fdiagnostics_parseable_fixits);

clang/lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   else
     for (const auto &A : XRayInstrBundles)
       parseXRayInstrumentationBundle("-fxray-instrumentation-bundle=", A, Args,
                                      Diags, Opts.XRayInstrumentationBundle);
 
   Opts.InstrumentForProfiling = Args.hasArg(OPT_pg);
   Opts.CallFEntry = Args.hasArg(OPT_mfentry);
   Opts.MNopMCount = Args.hasArg(OPT_mnop_mcount);
+  Opts.PackedStack = Args.hasArg(OPT_mpacked_stack);
   Opts.EmitOpenCLArgMetadata = Args.hasArg(OPT_cl_kernel_arg_info);
 
   if (const Arg *A = Args.getLastArg(OPT_fcf_protection_EQ)) {
     StringRef Name = A->getValue();
     if (Name == "full") {
       Opts.CFProtectionReturn = 1;
       Opts.CFProtectionBranch = 1;
     } else if (Name == "return")

clang/test/CodeGen/packed-stack.c

+// RUN: %clang_cc1 -mpacked-stack -triple s390x-ibm-linux -emit-llvm \
+// RUN:   -o - %s 2>&1 | FileCheck  %s
+// RUN: not %clang_cc1 -mpacked-stack -triple x86_64-linux-gnu \
+// RUN:   -emit-llvm -o - %s 2>&1 | FileCheck -check-prefix=X86 %s
+
+int foo(void) {
+  return 0;
+}
+
+//CHECK: attributes #0 = { {{.*}}"packed-stack"="true"{{.*}} }
+//X86: error: option '-mpacked-stack' cannot be specified on this target