Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RISCV] Store RVC and TSO ELF flags explicitly in RISCVTargetStreamer. NFCI #83344

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Feb 28, 2024

Instead of caching STI in the RISCVELFTargetStreamer, store the two flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR module metadata for LTO. So they need to be separated from the STI used to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of the STI that was used to construct the TargetStreamer between the constructor and the use of the flags.

…. NFCI

Instead of caching STI in the RISCVELFTargetStreamer, store the two
flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using
IR module metadata for LTO. So they need to be separated from the
STI used to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents
of the STI that was used to construct the TargetStreamer between
the constructor and the use of the flags.
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 28, 2024

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

Author: Craig Topper (topperc)

Changes

Instead of caching STI in the RISCVELFTargetStreamer, store the two flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR module metadata for LTO. So they need to be separated from the STI used to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of the STI that was used to construct the TargetStreamer between the constructor and the use of the flags.


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

4 Files Affected:

  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp (+4-4)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h (-1)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp (+6)
  • (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h (+5)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
index b375e8bb4b8fac..cdf7c048a4bf11 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -31,12 +31,13 @@ using namespace llvm;
 // This part is for ELF object output.
 RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
                                                const MCSubtargetInfo &STI)
-    : RISCVTargetStreamer(S), CurrentVendor("riscv"), STI(STI) {
+    : RISCVTargetStreamer(S), CurrentVendor("riscv") {
   MCAssembler &MCA = getStreamer().getAssembler();
   const FeatureBitset &Features = STI.getFeatureBits();
   auto &MAB = static_cast<RISCVAsmBackend &>(MCA.getBackend());
   setTargetABI(RISCVABI::computeTargetABI(STI.getTargetTriple(), Features,
                                           MAB.getTargetOptions().getABIName()));
+  setFlagsFromFeatures(STI);
   // `j label` in `.option norelax; j label; .option relax; ...; label:` needs a
   // relocation to ensure the jump target is correct after linking. This is due
   // to a limitation that shouldForceRelocation has to make the decision upfront
@@ -91,10 +92,9 @@ void RISCVTargetELFStreamer::finish() {
 
   unsigned EFlags = MCA.getELFHeaderEFlags();
 
-  if (STI.hasFeature(RISCV::FeatureStdExtC) ||
-      STI.hasFeature(RISCV::FeatureStdExtZca))
+  if (hasRVC())
     EFlags |= ELF::EF_RISCV_RVC;
-  if (STI.hasFeature(RISCV::FeatureStdExtZtso))
+  if (hasTSO())
     EFlags |= ELF::EF_RISCV_TSO;
 
   switch (ABI) {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
index a6f54bf67b5d2b..e8f29cd8449ba0 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
@@ -46,7 +46,6 @@ class RISCVTargetELFStreamer : public RISCVTargetStreamer {
   StringRef CurrentVendor;
 
   MCSection *AttributeSection = nullptr;
-  const MCSubtargetInfo &STI;
 
   void emitAttribute(unsigned Attribute, unsigned Value) override;
   void emitTextAttribute(unsigned Attribute, StringRef String) override;
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
index 071a3a5aa5d6e7..4a4b1e13c2b9ec 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
@@ -48,6 +48,12 @@ void RISCVTargetStreamer::setTargetABI(RISCVABI::ABI ABI) {
   TargetABI = ABI;
 }
 
+void RISCVTargetStreamer::setFlagsFromFeatures(const MCSubtargetInfo &STI) {
+  HasRVC = STI.hasFeature(RISCV::FeatureStdExtC) ||
+           STI.hasFeature(RISCV::FeatureStdExtZca);
+  HasTSO = STI.hasFeature(RISCV::FeatureStdExtZtso);
+}
+
 void RISCVTargetStreamer::emitTargetAttributes(const MCSubtargetInfo &STI,
                                                bool EmitStackAlign) {
   if (EmitStackAlign) {
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
index 070e72fb157ae9..cb8bc21cb63557 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
@@ -33,6 +33,8 @@ struct RISCVOptionArchArg {
 
 class RISCVTargetStreamer : public MCTargetStreamer {
   RISCVABI::ABI TargetABI = RISCVABI::ABI_Unknown;
+  bool HasRVC = false;
+  bool HasTSO = false;
 
 public:
   RISCVTargetStreamer(MCStreamer &S);
@@ -58,6 +60,9 @@ class RISCVTargetStreamer : public MCTargetStreamer {
   void emitTargetAttributes(const MCSubtargetInfo &STI, bool EmitStackAlign);
   void setTargetABI(RISCVABI::ABI ABI);
   RISCVABI::ABI getTargetABI() const { return TargetABI; }
+  void setFlagsFromFeatures(const MCSubtargetInfo &STI);
+  bool hasRVC() const { return HasRVC; }
+  bool hasTSO() const { return HasTSO; }
 };
 
 // This part is for ascii assembly output

Copy link
Contributor

@ilovepi ilovepi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@topperc topperc merged commit 6afda56 into llvm:main Feb 29, 2024
5 of 6 checks passed
@topperc topperc deleted the pr/elf-flags branch February 29, 2024 16:56
topperc added a commit to topperc/llvm-project that referenced this pull request May 8, 2024
…. NFCI (llvm#83344)

Instead of caching STI in the RISCVELFTargetStreamer, store the two
flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR
module metadata for LTO. So they need to be separated from the STI used
to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of
the STI that was used to construct the TargetStreamer between the
constructor and the use of the flags.
topperc added a commit to topperc/llvm-project that referenced this pull request May 8, 2024
…. NFCI (llvm#83344)

Instead of caching STI in the RISCVELFTargetStreamer, store the two
flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR
module metadata for LTO. So they need to be separated from the STI used
to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of
the STI that was used to construct the TargetStreamer between the
constructor and the use of the flags.
tstellar pushed a commit to topperc/llvm-project that referenced this pull request May 13, 2024
…. NFCI (llvm#83344)

Instead of caching STI in the RISCVELFTargetStreamer, store the two
flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR
module metadata for LTO. So they need to be separated from the STI used
to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of
the STI that was used to construct the TargetStreamer between the
constructor and the use of the flags.
tstellar pushed a commit to topperc/llvm-project that referenced this pull request May 14, 2024
…. NFCI (llvm#83344)

Instead of caching STI in the RISCVELFTargetStreamer, store the two
flags we need from it.

My goal is to allow RISCVAsmPrinter to override these flags using IR
module metadata for LTO. So they need to be separated from the STI used
to construct the TargetStreamer.

This patch should be NFC as long as no one is changing the contents of
the STI that was used to construct the TargetStreamer between the
constructor and the use of the flags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants