[lld] [llvm] [RISCV] Do not re-validate target ABI in AsmParser if already resolved by streamer (PR #223606)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: Alexander Richardson (arichardson)

<details>
<summary>Changes</summary>

Commit 105ff16f816f (https://github.com/llvm/llvm-project/pull/213410)
changed RISCVABI::computeTargetABI() to return Expected<ABI> and added
validation to RISCVAsmParser::onBeginOfFile() to report invalid
-target-abi flags with real source location in llvm-mc.

However, this broke LTO builds containing inline assembly (e.g. Android
riscv64 builds and downstream Rust in
https://github.com/rust-lang/rust/pull/162783). During LTO, LLD sets
TargetOptions.MCOptions.ABIName from the module's target-abi metadata
("lp64d"), while the linker's default TargetMachine subtarget lacks
"+d" (individual functions specify "+d" in target-features).
RISCVSubtarget handles this gracefully by emitting a diagnostic note
and falling back to lp64 for code generation. When inline assembly was
subsequently parsed, AsmPrinter::emitInlineAsm instantiated
RISCVAsmParser with a subtarget lacking "+d", causing onBeginOfFile() to
re-validate TargetOptions.ABIName and fail with a fatal error.

To fix this, skip the parser validation whenever the streamer has
already resolved an ABI. This preserves llvm-mc diagnostics on invalid
command-line flags while avoiding conflicting validation when assembling
inline asm during code generation.

This commit was created with the help of AI tools

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


5 Files Affected:

- (modified) lld/test/ELF/lto/riscv-target-abi.ll (+7) 
- (modified) llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp (+12-1) 
- (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp (+3-5) 
- (modified) llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h (+1) 
- (modified) llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp (+2) 


``````````diff
diff --git a/lld/test/ELF/lto/riscv-target-abi.ll b/lld/test/ELF/lto/riscv-target-abi.ll
index 07fdf122cb0c4..fdb93b82fc41e 100644
--- a/lld/test/ELF/lto/riscv-target-abi.ll
+++ b/lld/test/ELF/lto/riscv-target-abi.ll
@@ -28,7 +28,14 @@
 target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
 target triple = "riscv64"
 
+module asm "nop"
+;; Module asm with target features not including 'd' (would fail before fix)
+module asm(target_features: "+c") "c.nop"
+;; Module asm with target features enabling 'd'
+module asm(target_features: "+d") "fld f0, 0(sp)"
+
 define void @_start() {
+  call void asm sideeffect "nop", ""()
   ret void
 }
 
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 4f1f3cbfb9b26..d18f7c7416ac6 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -343,10 +343,21 @@ class RISCVAsmParser : public MCTargetAsmParser {
   // with the first token, so diagnostics can be reported with a real source
   // location instead of being printed with no location information.
   void onBeginOfFile() override {
+    // If the target streamer already has a resolved ABI (e.g. set by
+    // RISCVTargetELFStreamer for a valid -target-abi, or set by
+    // RISCVAsmPrinter during codegen), skip validation.
+    if (getTargetStreamer().hasTargetABI())
+      return;
+
     Expected<RISCVABI::ABI> ABIOrErr =
         RISCVABI::computeTargetABI(getSTI(), getTargetOptions().ABIName);
-    if (!ABIOrErr)
+    if (!ABIOrErr) {
       getParser().printError(getLoc(), toString(ABIOrErr.takeError()));
+      getTargetStreamer().setTargetABI(
+          cantFail(RISCVABI::computeTargetABI(getSTI(), "")));
+      return;
+    }
+    getTargetStreamer().setTargetABI(*ABIOrErr);
   }
 };
 
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
index 049123068070f..1fb45627a1d03 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -40,12 +40,10 @@ RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S,
   if (auto ABIOrErr = RISCVABI::computeTargetABI(STI, ABIName)) {
     setTargetABI(*ABIOrErr);
   } else {
-    // Do not warn here and instead silently fall back to the default ABI:
-    // either RISCVSubtarget::initializeSubtargetDependencies() or
-    // RISCVAsmParser::onBeginOfFile() will print the message with proper
-    // contexts. Reporting here would just duplicate that diagnostic.
+    // Do not set TargetABI here if invalid: RISCVSubtarget/RISCVAsmPrinter
+    // (in codegen) or RISCVAsmParser::onBeginOfFile() (in llvm-mc) will
+    // resolve or diagnose it with proper contexts.
     consumeError(ABIOrErr.takeError());
-    setTargetABI(cantFail(RISCVABI::computeTargetABI(STI, "")));
   }
   setFlagsFromFeatures(STI);
 
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
index 0601cf0ed0362..cd59089fae4c7 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
@@ -63,6 +63,7 @@ class RISCVTargetStreamer : public MCTargetStreamer {
   void emitTargetAttributes(const MCSubtargetInfo &STI, bool EmitStackAlign);
   void setTargetABI(RISCVABI::ABI ABI);
   RISCVABI::ABI getTargetABI() const { return TargetABI; }
+  bool hasTargetABI() const { return TargetABI != RISCVABI::ABI_Unknown; }
   void setFlagsFromFeatures(const MCSubtargetInfo &STI);
   bool hasRVC() const { return HasRVC; }
   bool hasTSO() const { return HasTSO; }
diff --git a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
index b535404d25c1f..57fb1dfc48fec 100644
--- a/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
+++ b/llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
@@ -649,6 +649,8 @@ void RISCVAsmPrinter::emitStartOfAsmFile(Module &M) {
   if (const MDString *ModuleTargetABI =
           dyn_cast_or_null<MDString>(M.getModuleFlag("target-abi")))
     RTS.setTargetABI(RISCVABI::getTargetABI(ModuleTargetABI->getString()));
+  else if (!RTS.hasTargetABI())
+    RTS.setTargetABI(TM.getSubtarget<RISCVSubtarget>().getTargetABI());
 
   MCSubtargetInfo SubtargetInfo = TM.getMCSubtargetInfo();
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/223606