[llvm] [RISCV] Collect function features in AsmPrinter before emission (#76231) (PR #76437)

Andreu Carminati via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 28 05:29:19 PST 2023


https://github.com/andcarminati updated https://github.com/llvm/llvm-project/pull/76437

>From dd7704643d9ee14ade78477de6d2a37dd419aba3 Mon Sep 17 00:00:00 2001
From: Andreu Carminati <andreu.carminati at hightec-rt.com>
Date: Wed, 27 Dec 2023 11:19:06 +0100
Subject: [PATCH 1/2] [RISCV] Collect function features in AsmPrinter before
 emission (#76231)

When performing LTO, if we use just TM MCSubtargetInfo feature flags, it will
be incomplete because these informations will be in function's Subtarget.
In this way, we collect the flags of all functions and postpone the attribute
emission to the end of the file emission.
---
 llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp     | 32 ++++++++++++++-----
 .../CodeGen/RISCV/flags-collect-asmprinter.ll | 28 ++++++++++++++++
 2 files changed, 52 insertions(+), 8 deletions(-)
 create mode 100644 llvm/test/CodeGen/RISCV/flags-collect-asmprinter.ll

diff --git a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
index 0fd514fa87cd2f..75224bbbb62524 100644
--- a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -54,11 +54,16 @@ extern const SubtargetFeatureKV RISCVFeatureKV[RISCV::NumSubtargetFeatures];
 namespace {
 class RISCVAsmPrinter : public AsmPrinter {
   const RISCVSubtarget *STI;
+  std::unique_ptr<MCSubtargetInfo> CommonSTI;
 
 public:
   explicit RISCVAsmPrinter(TargetMachine &TM,
                            std::unique_ptr<MCStreamer> Streamer)
-      : AsmPrinter(TM, std::move(Streamer)) {}
+      : AsmPrinter(TM, std::move(Streamer)) {
+    std::unique_ptr<MCSubtargetInfo> STIPtr(
+        TM.getTarget().createMCSubtargetInfo("", "", ""));
+    CommonSTI = std::move(STIPtr);
+  }
 
   StringRef getPassName() const override { return "RISC-V Assembly Printer"; }
 
@@ -71,6 +76,8 @@ class RISCVAsmPrinter : public AsmPrinter {
   void LowerSTATEPOINT(MCStreamer &OutStreamer, StackMaps &SM,
                        const MachineInstr &MI);
 
+  bool doInitialization(Module &M) override;
+
   bool runOnMachineFunction(MachineFunction &MF) override;
 
   void emitInstruction(const MachineInstr *MI) override;
@@ -363,6 +370,12 @@ bool RISCVAsmPrinter::emitDirectiveOptionArch() {
   return false;
 }
 
+bool RISCVAsmPrinter::doInitialization(Module &M) {
+
+  CommonSTI->setFeatureBits(TM.getMCSubtargetInfo()->getFeatureBits());
+  return AsmPrinter::doInitialization(M);
+}
+
 bool RISCVAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
   STI = &MF.getSubtarget<RISCVSubtarget>();
   RISCVTargetStreamer &RTS =
@@ -370,6 +383,10 @@ bool RISCVAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
 
   bool EmittedOptionArch = emitDirectiveOptionArch();
 
+  // Collect flags from this function.
+  CommonSTI->setFeatureBits(CommonSTI->getFeatureBits() |
+                            STI->getFeatureBits());
+
   SetupMachineFunction(MF);
   emitFunctionBody();
 
@@ -384,26 +401,25 @@ void RISCVAsmPrinter::emitStartOfAsmFile(Module &M) {
   if (const MDString *ModuleTargetABI =
           dyn_cast_or_null<MDString>(M.getModuleFlag("target-abi")))
     RTS.setTargetABI(RISCVABI::getTargetABI(ModuleTargetABI->getString()));
-  if (TM.getTargetTriple().isOSBinFormatELF())
-    emitAttributes();
 }
 
 void RISCVAsmPrinter::emitEndOfAsmFile(Module &M) {
   RISCVTargetStreamer &RTS =
       static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
 
-  if (TM.getTargetTriple().isOSBinFormatELF())
+  if (TM.getTargetTriple().isOSBinFormatELF()) {
+    emitAttributes();
     RTS.finishAttributeSection();
+  }
+
   EmitHwasanMemaccessSymbols(M);
 }
 
 void RISCVAsmPrinter::emitAttributes() {
   RISCVTargetStreamer &RTS =
       static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
-  // Use MCSubtargetInfo from TargetMachine. Individual functions may have
-  // attributes that differ from other functions in the module and we have no
-  // way to know which function is correct.
-  RTS.emitTargetAttributes(*TM.getMCSubtargetInfo(), /*EmitStackAlign*/ true);
+  // Use MCSubtargetInfo with flags collected from all functions.
+  RTS.emitTargetAttributes(*CommonSTI.get(), /*EmitStackAlign*/ true);
 }
 
 void RISCVAsmPrinter::emitFunctionEntryLabel() {
diff --git a/llvm/test/CodeGen/RISCV/flags-collect-asmprinter.ll b/llvm/test/CodeGen/RISCV/flags-collect-asmprinter.ll
new file mode 100644
index 00000000000000..6a0758eeb6e2d1
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/flags-collect-asmprinter.ll
@@ -0,0 +1,28 @@
+; RUN: llc -mtriple=riscv32  -verify-machineinstrs < %s -filetype=obj -o %t
+; RUN: llvm-readelf -A %t | FileCheck %s
+
+; CHECK: Value: rv32i2p1_m2p0_a2p1_f2p2_d2p2_c2p0_zicsr2p0
+define dso_local i32 @func0() #0 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func1() #1 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func2() #2 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func3() #3 {
+entry:
+  ret i32 0
+}
+
+attributes #0 = { nounwind "target-features"="+32bit,+d,+zicsr" }
+attributes #1 = { nounwind "target-features"="+32bit,+d,+f,+m" }
+attributes #2 = { nounwind "target-features"="+32bit,+f,+c" }
+attributes #3 = { nounwind "target-features"="+32bit,+a" }

>From e6b0655afa925dba283d3fba5dd94310cd4ec354 Mon Sep 17 00:00:00 2001
From: Andreu Carminati <andreu.carminati at hightec-rt.com>
Date: Thu, 28 Dec 2023 14:24:01 +0100
Subject: [PATCH 2/2] Filter invalid feature set combinations

Addressing reviews.
---
 .../RISCV/MCTargetDesc/RISCVBaseInfo.cpp      | 33 ++++++++++++++++++
 .../Target/RISCV/MCTargetDesc/RISCVBaseInfo.h |  2 ++
 llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp     |  3 ++
 .../flags-collect-asmprinter-incompatible.ll  | 34 +++++++++++++++++++
 4 files changed, 72 insertions(+)
 create mode 100644 llvm/test/CodeGen/RISCV/flags-collect-asmprinter-incompatible.ll

diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
index 66a46a485f5388..90cded81407494 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
@@ -128,6 +128,39 @@ parseFeatureBits(bool IsRV64, const FeatureBitset &FeatureBits) {
   return llvm::RISCVISAInfo::parseFeatures(XLen, FeatureVector);
 }
 
+// TODO: can we encode this in tblgen?
+// 3rd element cannot fit with the previous ones, or the same as previous,
+// if we don't have this restriction of A + B cannot fit with C.
+static const std::tuple<unsigned int, unsigned int, unsigned int>
+    IncompatibleFeatures[] = {{RISCV::FeatureStdExtF, RISCV::FeatureStdExtZfinx,
+                               // Repeat.
+                               RISCV::FeatureStdExtZfinx},
+                              {RISCV::FeatureStdExtD, RISCV::FeatureStdExtZcmp,
+                               // C cannot be with D + Zcmp.
+                               RISCV::FeatureStdExtC},
+                              {RISCV::FeatureStdExtD, RISCV::FeatureStdExtZcmt,
+                               // C cannot be with D + Zcmt.
+                               RISCV::FeatureStdExtC}};
+
+void filterOffIncompatibleFeatureBits(MCSubtargetInfo *STI) {
+  const auto &Features = STI->getFeatureBits();
+  // We cannot test and reset the same set,
+  // otherwise we will hide some combinations.
+  auto FeaturesToReset = STI->getFeatureBits();
+  for (auto &FeatTuple : IncompatibleFeatures) {
+    auto FirstFlag = std::get<0>(FeatTuple);
+    auto SecondFlag = std::get<1>(FeatTuple);
+    auto ThirdFlag = std::get<2>(FeatTuple);
+    if (Features.test(FirstFlag) && Features.test(SecondFlag) &&
+        Features.test(ThirdFlag)) {
+      FeaturesToReset.reset(FirstFlag);
+      FeaturesToReset.reset(SecondFlag);
+      FeaturesToReset.reset(ThirdFlag);
+    }
+  }
+  STI->setFeatureBits(FeaturesToReset);
+}
+
 } // namespace RISCVFeatures
 
 // Encode VTYPE into the binary format used by the the VSETVLI instruction which
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
index 30ed36525e29f2..13c32185aff6ab 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
@@ -491,6 +491,8 @@ void validate(const Triple &TT, const FeatureBitset &FeatureBits);
 llvm::Expected<std::unique_ptr<RISCVISAInfo>>
 parseFeatureBits(bool IsRV64, const FeatureBitset &FeatureBits);
 
+void filterOffIncompatibleFeatureBits(MCSubtargetInfo *STI);
+
 } // namespace RISCVFeatures
 
 namespace RISCVVType {
diff --git a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
index 75224bbbb62524..a4c9b04e5c12d8 100644
--- a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -408,6 +408,9 @@ void RISCVAsmPrinter::emitEndOfAsmFile(Module &M) {
       static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
 
   if (TM.getTargetTriple().isOSBinFormatELF()) {
+    // We are interested only in the set of common and compatible
+    // flags accross all functions.
+    RISCVFeatures::filterOffIncompatibleFeatureBits(CommonSTI.get());
     emitAttributes();
     RTS.finishAttributeSection();
   }
diff --git a/llvm/test/CodeGen/RISCV/flags-collect-asmprinter-incompatible.ll b/llvm/test/CodeGen/RISCV/flags-collect-asmprinter-incompatible.ll
new file mode 100644
index 00000000000000..c9277a1f037623
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/flags-collect-asmprinter-incompatible.ll
@@ -0,0 +1,34 @@
+; RUN: llc -mtriple=riscv32  -verify-machineinstrs < %s -filetype=obj -o %t
+; RUN: llvm-readelf -A %t | FileCheck %s
+
+; CHECK: Value:  rv32i2p1_a2p1_zicsr2p0_zca1p0
+define dso_local i32 @func0() #0 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func1() #1 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func2() #2 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func3() #3 {
+entry:
+  ret i32 0
+}
+
+define dso_local i32 @func4() #4 {
+entry:
+  ret i32 0
+}
+
+attributes #0 = { nounwind "target-features"="+32bit,+d" }
+attributes #1 = { nounwind "target-features"="+32bit,+c" }
+attributes #2 = { nounwind "target-features"="+32bit,+f,+zcmp" }
+attributes #3 = { nounwind "target-features"="+32bit,+a,+zfinx" }
+attributes #4 = { nounwind "target-features"="+32bit,+a,+zcmt" }



More information about the llvm-commits mailing list