Set MAttrs in LTO mode

[yinma]

Summary

Without this patch, MAttrs are not set.

[pcc]

The MAttrs part is fine but we don't want CGOptLevel to be set based on the LTO opt level but rather on the opt level passed at compile time. See mailing list thread "[RFC] Adding function attributes to represent codegen optimization level" for more information.

[yinma]

I see you point. I have removed CGOptLevel portion

[MaskRay]

Common/TargetOptionsCommandFlags.cpp

36 ↗

(On Diff #170235)

return std::vector<std::string>(MAttrs.begin(), MAttrs.end());

[ruiu]

Common/TargetOptionsCommandFlags.cpp

38 ↗

(On Diff #170235)

Where is this MAttrs defined?

[yinma]

Common/TargetOptionsCommandFlags.cpp

36 ↗

(On Diff #170235)

OK

38 ↗

(On Diff #170235)

Those can be set by commend line options

[ruiu]

Common/TargetOptionsCommandFlags.cpp

38 ↗

(On Diff #170235)

But I can't find that code in this patch. Is that is another patch?

[yinma]

Common/TargetOptionsCommandFlags.cpp

38 ↗

(On Diff #170235)

I see what you are asking for. it is defined in LLVM.

include/llvm/CodeGen/CommandFlags.inc:40: MAttrs("mattr", cl::CommaSeparated,

[ruiu]

Common/TargetOptionsCommandFlags.cpp

36 ↗

(On Diff #170235)

Please format carefully -- you need a space between () and the following {. I'd recommend clang-format-diff if you are not using it already.

38 ↗

(On Diff #170235)

Oh, I missed that. So MAttrs is not even in any namespace? Not sure if that is a good practice. Anyway, please add a comment saying that "MAttrs is defined in llvm/CodeGen/CommandFlags.inc."

I believe you can simply return MAttrs from this function.

[ruiu]

Common/TargetOptionsCommandFlags.cpp

38 ↗

(On Diff #170235)

Or, maybe the easiest way of writing this function is just this:

std::vector<std::string> lld::GetMAttrs() { return ::MAttrs; }

[ruiu]

ELF/LTO.cpp

91–92 ↗

(On Diff #170235)

I don't think we need this comment as we don't have one for GetCPUStr() and others. For lld, they are just opaque arguments passed from users to the LTO backend.

[MaskRay]

COFF/LTO.cpp

63 ↗

(On Diff #171379)

Delete the empty line?

[pcc]

LGTM

[MaskRay]

I'll commit on behalf of @yinma as he contacted me to do that.

lld/trunk/COFF/LTO.cpp

static std::unique_ptr<lto::LTO> createLTO() {
   // using the PIC model (see PR34306).
   if (Config->Machine == COFF::IMAGE_FILE_MACHINE_I386)
     C.RelocModel = Reloc::Static;
   else
     C.RelocModel = Reloc::PIC_;
   C.DisableVerify = true;
   C.DiagHandler = diagnosticHandler;
   C.OptLevel = Config->LTOO;
+  C.MAttrs = GetMAttrs();
+
   if (Config->SaveTemps)
     checkError(C.addSaveTemps(std::string(Config->OutputFile) + ".",
                               /*UseInputModulePath*/ true));
   lto::ThinBackend Backend;
   if (Config->ThinLTOJobs != 0)
     Backend = lto::createInProcessThinBackend(Config->ThinLTOJobs);
   return llvm::make_unique<lto::LTO>(std::move(C), Backend,
                                      Config->LTOPartitions);

lld/trunk/Common/TargetOptionsCommandFlags.cpp

llvm::TargetOptions lld::InitTargetOptionsFromCodeGenFlags() {
   return ::InitTargetOptionsFromCodeGenFlags();
 }
 
 llvm::Optional<llvm::CodeModel::Model> lld::GetCodeModelFromCMModel() {
   return getCodeModel();
 }
 
 std::string lld::GetCPUStr() { return ::getCPUStr(); }
+std::vector<std::string> lld::GetMAttrs() { return ::MAttrs; }

lld/trunk/ELF/LTO.cpp

static lto::Config createConfig() {
   else
     C.RelocModel = Reloc::Static;
 
   C.CodeModel = GetCodeModelFromCMModel();
   C.DisableVerify = Config->DisableVerify;
   C.DiagHandler = diagnosticHandler;
   C.OptLevel = Config->LTOO;
   C.CPU = GetCPUStr();
+  C.MAttrs = GetMAttrs();
 
   // Set up a custom pipeline if we've been asked to.
   C.OptPipeline = Config->LTONewPmPasses;
   C.AAPipeline = Config->LTOAAPipeline;
 
   // Set up optimization remarks if we've been asked to.
   C.RemarksFilename = Config->OptRemarksFilename;
   C.RemarksWithHotness = Config->OptRemarksWithHotness;

lld/trunk/include/lld/Common/TargetOptionsCommandFlags.h

 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/CodeGen.h"
 #include "llvm/Target/TargetOptions.h"
 
 namespace lld {
 llvm::TargetOptions InitTargetOptionsFromCodeGenFlags();
 llvm::Optional<llvm::CodeModel::Model> GetCodeModelFromCMModel();
 std::string GetCPUStr();
+std::vector<std::string> GetMAttrs();
 }

lld/trunk/wasm/LTO.cpp

static std::unique_ptr<lto::LTO> createLTO() {
   C.Options.DataSections = true;
 
   // Wasm currently only supports ThreadModel::Single
   C.Options.ThreadModel = ThreadModel::Single;
 
   C.DisableVerify = Config->DisableVerify;
   C.DiagHandler = diagnosticHandler;
   C.OptLevel = Config->LTOO;
+  C.MAttrs = GetMAttrs();
 
   if (Config->SaveTemps)
     checkError(C.addSaveTemps(Config->OutputFile.str() + ".",
                               /*UseInputModulePath*/ true));
 
   lto::ThinBackend Backend;
   if (Config->ThinLTOJobs != -1U)
     Backend = lto::createInProcessThinBackend(Config->ThinLTOJobs);