Use Module level target-abi to assign target features for codegenerated functions.

[hiraditya]

It is currently not possible to provide any reasonable target-features for compiler generated functions (See: #69780). Having a target-abi will provide a way to add minimal requirements for target-features like +d for RISC-V.

[llvmbot]

@llvm/pr-subscribers-llvm-ir
@llvm/pr-subscribers-backend-risc-v
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-lto

Author: AdityaK (hiraditya)

Changes

Module can be used to query target-abi (follow up patch) which can be used to populate default subtarget features. It is currently not possible to provide any reasonable target-features for compiler generated functions (See: #69780) Having a target-abi will provide a way to add minimal requirements for target-features like +d for RISC-V.

---

Full diff: https://github.com/llvm/llvm-project/pull/100833.diff

7 Files Affected:

• (modified) llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h (+1-1)
• (modified) llvm/include/llvm/TargetParser/SubtargetFeature.h (+2-1)
• (modified) llvm/lib/LTO/LTOBackend.cpp (+1-1)
• (modified) llvm/lib/LTO/LTOCodeGenerator.cpp (+1-1)
• (modified) llvm/lib/LTO/LTOModule.cpp (+1-1)
• (modified) llvm/lib/LTO/ThinLTOCodeGenerator.cpp (+7-5)
• (modified) llvm/lib/TargetParser/SubtargetFeature.cpp (+2-1)

diff --git a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
index f1337e82485c9..50375f701cecf 100644
--- a/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
+++ b/llvm/include/llvm/LTO/legacy/ThinLTOCodeGenerator.h
@@ -40,7 +40,7 @@ struct TargetMachineBuilder {
   std::optional<Reloc::Model> RelocModel;
   CodeGenOptLevel CGOptLevel = CodeGenOptLevel::Aggressive;
 
-  std::unique_ptr<TargetMachine> create() const;
+  std::unique_ptr<TargetMachine> create(const Module &M) const;
 };
 
 /// This class define an interface similar to the LTOCodeGenerator, but adapted
diff --git a/llvm/include/llvm/TargetParser/SubtargetFeature.h b/llvm/include/llvm/TargetParser/SubtargetFeature.h
index 2e1f00dad2df3..d75db9210495b 100644
--- a/llvm/include/llvm/TargetParser/SubtargetFeature.h
+++ b/llvm/include/llvm/TargetParser/SubtargetFeature.h
@@ -20,6 +20,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/IR/Module.h"
 #include "llvm/Support/MathExtras.h"
 #include <array>
 #include <initializer_list>
@@ -195,7 +196,7 @@ class SubtargetFeatures {
   void dump() const;
 
   /// Adds the default features for the specified target triple.
-  void getDefaultSubtargetFeatures(const Triple& Triple);
+  void getDefaultSubtargetFeatures(const Module &M);
 
   /// Determine if a feature has a flag; '+' or '-'
   static bool hasFlag(StringRef Feature) {
diff --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index d5d642f0d25e6..43fab8c941e77 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -201,7 +201,7 @@ static std::unique_ptr<TargetMachine>
 createTargetMachine(const Config &Conf, const Target *TheTarget, Module &M) {
   StringRef TheTriple = M.getTargetTriple();
   SubtargetFeatures Features;
-  Features.getDefaultSubtargetFeatures(Triple(TheTriple));
+  Features.getDefaultSubtargetFeatures(M);
   for (const std::string &A : Conf.MAttrs)
     Features.AddFeature(A);
 
diff --git a/llvm/lib/LTO/LTOCodeGenerator.cpp b/llvm/lib/LTO/LTOCodeGenerator.cpp
index 34aacb660144f..18826d0c78f1f 100644
--- a/llvm/lib/LTO/LTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -406,7 +406,7 @@ bool LTOCodeGenerator::determineTarget() {
   // Construct LTOModule, hand over ownership of module and target. Use MAttr as
   // the default set of features.
   SubtargetFeatures Features(join(Config.MAttrs, ""));
-  Features.getDefaultSubtargetFeatures(Triple);
+  Features.getDefaultSubtargetFeatures(*MergedModule);
   FeatureStr = Features.getString();
   if (Config.CPU.empty())
     Config.CPU = lto::getThinLTODefaultCPU(Triple);
diff --git a/llvm/lib/LTO/LTOModule.cpp b/llvm/lib/LTO/LTOModule.cpp
index eac78069f4d2b..e77c4a7118628 100644
--- a/llvm/lib/LTO/LTOModule.cpp
+++ b/llvm/lib/LTO/LTOModule.cpp
@@ -213,7 +213,7 @@ LTOModule::makeLTOModule(MemoryBufferRef Buffer, const TargetOptions &options,
 
   // construct LTOModule, hand over ownership of module and target
   SubtargetFeatures Features;
-  Features.getDefaultSubtargetFeatures(Triple);
+  Features.getDefaultSubtargetFeatures(*M.get());
   std::string FeatureStr = Features.getString();
   // Set a default CPU for Darwin triples.
   std::string CPU;
diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
index 57c94e1988b79..62b58de3ddbdd 100644
--- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
+++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp
@@ -582,7 +582,7 @@ void ThinLTOCodeGenerator::crossReferenceSymbol(StringRef Name) {
 }
 
 // TargetMachine factory
-std::unique_ptr<TargetMachine> TargetMachineBuilder::create() const {
+std::unique_ptr<TargetMachine> TargetMachineBuilder::create(const Module &M) const {
   std::string ErrMsg;
   const Target *TheTarget =
       TargetRegistry::lookupTarget(TheTriple.str(), ErrMsg);
@@ -592,7 +592,7 @@ std::unique_ptr<TargetMachine> TargetMachineBuilder::create() const {
 
   // Use MAttr as the default set of features.
   SubtargetFeatures Features(MAttr);
-  Features.getDefaultSubtargetFeatures(TheTriple);
+  Features.getDefaultSubtargetFeatures(M);
   std::string FeatureStr = Features.getString();
 
   std::unique_ptr<TargetMachine> TM(
@@ -920,7 +920,7 @@ void ThinLTOCodeGenerator::optimize(Module &TheModule) {
   initTMBuilder(TMBuilder, Triple(TheModule.getTargetTriple()));
 
   // Optimize now
-  optimizeModule(TheModule, *TMBuilder.create(), OptLevel, Freestanding,
+  optimizeModule(TheModule, *TMBuilder.create(TheModule), OptLevel, Freestanding,
                  DebugPassManager, nullptr);
 }
 
@@ -997,7 +997,8 @@ void ThinLTOCodeGenerator::run() {
                                              /*IsImporting*/ false);
 
         // CodeGen
-        auto OutputBuffer = codegenModule(*TheModule, *TMBuilder.create());
+        TargetMachine &TM = *TMBuilder.create(*TheModule);
+        auto OutputBuffer = codegenModule(*TheModule, TM);
         if (SavedObjectsDirectoryPath.empty())
           ProducedBinaries[count] = std::move(OutputBuffer);
         else
@@ -1185,9 +1186,10 @@ void ThinLTOCodeGenerator::run() {
         saveTempBitcode(*TheModule, SaveTempsDir, count, ".0.original.bc");
 
         auto &ImportList = ImportLists[ModuleIdentifier];
+        TargetMachine &TM = *TMBuilder.create(*TheModule);
         // Run the main process now, and generates a binary
         auto OutputBuffer = ProcessThinLTOModule(
-            *TheModule, *Index, ModuleMap, *TMBuilder.create(), ImportList,
+            *TheModule, *Index, ModuleMap, TM, ImportList,
             ExportList, GUIDPreservedSymbols,
             ModuleToDefinedGVSummaries[ModuleIdentifier], CacheOptions,
             DisableCodeGen, SaveTempsDir, Freestanding, OptLevel, count,
diff --git a/llvm/lib/TargetParser/SubtargetFeature.cpp b/llvm/lib/TargetParser/SubtargetFeature.cpp
index 2c51c403c1934..e1fd039199bf1 100644
--- a/llvm/lib/TargetParser/SubtargetFeature.cpp
+++ b/llvm/lib/TargetParser/SubtargetFeature.cpp
@@ -68,7 +68,8 @@ LLVM_DUMP_METHOD void SubtargetFeatures::dump() const {
 }
 #endif
 
-void SubtargetFeatures::getDefaultSubtargetFeatures(const Triple& Triple) {
+void SubtargetFeatures::getDefaultSubtargetFeatures(const Module &M) {
+  llvm::Triple Triple(M.getTargetTriple());
   // FIXME: This is an inelegant way of specifying the features of a
   // subtarget. It would be better if we could encode this information
   // into the IR.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[echristo]

I need to take a closer look at this, but wanted to let you know I am reviewing. :)

[ilovepi]

Left some small nits and questions. This mostly seems fine, but lets see if anyone more experienced w/ the ABI features disagrees or sees an obvious foot-gun.

Off the top of my head @topperc has done some work in this area. Maybe he can suggest another person to check in with?

[ilovepi]

Do we have stability guarantees about the target-abi metadata? I don't think we do, but I also don't recall any changes around it... Is relying on parsing the string stable?

[topperc]

Should we check that the triple is RISCV?

[hiraditya]

Do we have stability guarantees about the target-abi metadata? I don't think we do, but I also don't recall any changes around it... Is relying on parsing the string stable?

I think there was a patch that added lp* information as metadata in lto/thinlto. So far all the examples I saw also had this information but I can look up the patch if that is needed. At least for the use case I'm looking at this change should be sufficient.

[hiraditya]

Should we check that the triple is RISCV?

I'm okay with it. Preparing the patch now.

[topperc]

Should we check that the triple is RISCV?

[ilovepi]

LGTM.

[topperc]

LGTM

[echristo]

So we cache a bunch of module stuff in the TargetMachine itself. Is there a reason why we wouldn't want to do the same with the abi string? i.e. I'm not sure we need to propagate the entire module down. Also do you have some code that uses this offhand that we can take a look at? I'd like to see the API use if that makes sense?

Thanks!

[topperc]

The ABI might be in the TargetMachine via Options.MCOptions.getABIName().

RISCVTargetMachine::getSubtargetImpl checks that the module ABI string matches it.