[llvm-branch-commits] [llvm] [RISC-V][MC] Fix mapping symbol section tracking on popSection() (PR #225131)
Alexander Richardson via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Sat Sep 26 19:01:46 PDT 2026
https://github.com/arichardson updated https://github.com/llvm/llvm-project/pull/225131
>From ea291d0c0beb2b2e945c31fa84e11aef979fdc90 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Mon, 21 Sep 2026 23:42:19 -0700
Subject: [PATCH 1/3] [LTO] Preserve module inline asm target properties for
.lto_discard and symvers
Previously, LTO::addRegularLTO() and IRLinker::run() called
prependModuleInlineAsm() and appendModuleInlineAsm() with a plain string when
synthesizing `.lto_discard` and imported `.symver` directives, creating a new
GlobalAsmFragment with empty TargetCPU and TargetFeatures instead of preserving
the existing module inline asm's properties. Copy the front fragment's Props so
these synthesized directives are merged into the module's inline asm with the
same target features.
This commit was created with the help of AI tools
---
cross-project-tests/riscv/lto-inline-asm-abi.c | 7 ++-----
llvm/lib/LTO/LTO.cpp | 2 +-
llvm/lib/Linker/IRMover.cpp | 3 ++-
llvm/test/LTO/RISCV/module-asm.ll | 5 +----
4 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/cross-project-tests/riscv/lto-inline-asm-abi.c b/cross-project-tests/riscv/lto-inline-asm-abi.c
index e89e1e4f0687d..883244f1854f7 100644
--- a/cross-project-tests/riscv/lto-inline-asm-abi.c
+++ b/cross-project-tests/riscv/lto-inline-asm-abi.c
@@ -22,15 +22,12 @@
// RUN: llvm-objdump -d --show-all-symbols --no-show-raw-insn %t.thin.so | FileCheck %s --check-prefix=DISASM
// RUN: llvm-objdump -t %t.thin.so | FileCheck %s --check-prefix=SYMS --implicit-check-not='\$x'
//
-/// TODO: LTO::addRegularLTO and IRLinker::run drop target_features and
-/// target_cpu when synthesizing .lto_discard and imported .symver directives.
-// REGULAR-IR: module asm{{$}}
+// REGULAR-IR: module asm(target_features: "+64bit,{{.*}}", target_cpu: "generic-rv64")
// REGULAR-IR-NEXT: ".lto_discard "
-// REGULAR-IR-NEXT: module asm(target_features: "+64bit,{{.*}}", target_cpu: "generic-rv64")
// REGULAR-IR-NEXT: "nop"
// REGULAR-IR-NEXT: ".symver symver_fn, symver_fn at VER_1.0"
//
-// THIN-IR: module asm{{$}}
+// THIN-IR: module asm(target_features: "+64bit,{{.*}}", target_cpu: "generic-rv64")
// THIN-IR-NEXT: ".symver symver_fn, symver_fn at VER_1.0"
//
// FLAGS: Flags [ (0x5)
diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp
index 4594c52fb5f6e..e307b7f1c16e8 100644
--- a/llvm/lib/LTO/LTO.cpp
+++ b/llvm/lib/LTO/LTO.cpp
@@ -1129,7 +1129,7 @@ LTO::addRegularLTO(InputFile &Input, ArrayRef<SymbolResolution> InputRes,
NewIA += " " + llvm::join(NonPrevailingAsmSymbols, ", ");
}
NewIA += "\n";
- M.prependModuleInlineAsm(NewIA);
+ M.prependModuleInlineAsm({NewIA, M.getModuleInlineAsm().front().Props});
}
assert(MsymI == MsymE);
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 3b72b412d0b2e..080ba6d542f71 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1576,7 +1576,8 @@ Error IRLinker::run() {
S += Name;
S += ", ";
S += Alias;
- DstM.appendModuleInlineAsm(std::string(S));
+ DstM.appendModuleInlineAsm(
+ {std::string(S), SrcM->getModuleInlineAsm().front().Props});
}
});
}
diff --git a/llvm/test/LTO/RISCV/module-asm.ll b/llvm/test/LTO/RISCV/module-asm.ll
index e213ec14a2a58..f27943f5457ac 100644
--- a/llvm/test/LTO/RISCV/module-asm.ll
+++ b/llvm/test/LTO/RISCV/module-asm.ll
@@ -7,11 +7,8 @@
; NM: T func
-;; TODO: LTO::addRegularLTO prepends ".lto_discard" without preserving the
-;; existing module inline asm's TargetCPU and TargetFeatures.
-; IR: module asm
+; IR: module asm(target_features: "+d")
; IR-NEXT: ".lto_discard"
-; IR-NEXT: module asm(target_features: "+d")
; IR-NEXT: ".globl func"
; IR-NEXT: "func:"
; IR-NEXT: "fld f0, 0(sp)"
>From 7579da8c326084f20a3fc6df8df7dd5b1302f4b4 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Tue, 22 Sep 2026 22:52:24 -0700
Subject: [PATCH 2/3] address feedback
---
llvm/lib/Linker/IRMover.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/lib/Linker/IRMover.cpp b/llvm/lib/Linker/IRMover.cpp
index 080ba6d542f71..d96c5d18a0ae3 100644
--- a/llvm/lib/Linker/IRMover.cpp
+++ b/llvm/lib/Linker/IRMover.cpp
@@ -1577,7 +1577,7 @@ Error IRLinker::run() {
S += ", ";
S += Alias;
DstM.appendModuleInlineAsm(
- {std::string(S), SrcM->getModuleInlineAsm().front().Props});
+ {std::string(S), SrcM->getModuleInlineAsm().back().Props});
}
});
}
>From 24f91e6dd179589165ac8942027a954b4d651003 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Mon, 21 Sep 2026 08:59:32 -0700
Subject: [PATCH 3/3] [RISC-V][MC] Fix mapping symbol section tracking on
popSection()
Previously, RISCVELFStreamer::changeSection() saved LastEMS and LastEmittedArch
under getPreviousSection().first instead of getCurrentSection().first. When
MCStreamer::popSection() switches back to a previous section,
getPreviousSection() already points to the destination section being restored
rather than the section being exited. This clobbered the destination section's
saved mapping symbol state and caused duplicate `$x<arch>` mapping symbols to
be emitted whenever returning to `.text`.
Use getCurrentSection().first instead, matching AArch64ELFStreamer and
ARMELFStreamer.
This commit was created with the help of AI tools
---
llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp | 6 +++---
llvm/test/MC/RISCV/mapping-across-sections.s | 7 +------
2 files changed, 4 insertions(+), 9 deletions(-)
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
index 524d0bb79d5f8..10702a836de33 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
@@ -218,9 +218,9 @@ void RISCVELFStreamer::changeSection(MCSection *Section, uint32_t Subsection) {
// default constructor by DenseMap::lookup. The last ISA suffix emitted in
// each section is also preserved so that re-entering a section only emits a
// new "$x<ISA>" symbol when the active ISA has actually changed.
- const MCSection *Prev = getPreviousSection().first;
- LastMappingSymbols[Prev] = LastEMS;
- LastEmittedArchInSection[Prev] = LastEmittedArch;
+ const MCSection *Cur = getCurrentSection().first;
+ LastMappingSymbols[Cur] = LastEMS;
+ LastEmittedArchInSection[Cur] = LastEmittedArch;
LastEMS = LastMappingSymbols.lookup(Section);
auto It = LastEmittedArchInSection.find(Section);
LastEmittedArch = It != LastEmittedArchInSection.end() ? It->second : "";
diff --git a/llvm/test/MC/RISCV/mapping-across-sections.s b/llvm/test/MC/RISCV/mapping-across-sections.s
index 9a741e792a244..54c420085da12 100644
--- a/llvm/test/MC/RISCV/mapping-across-sections.s
+++ b/llvm/test/MC/RISCV/mapping-across-sections.s
@@ -35,10 +35,7 @@
# CHECK: [[#WIBBLE:]]] .wibble
# CHECK: [[#STARTS_DATA:]]] .starts_data
-## TODO: RISCVELFStreamer::changeSection saves mapping symbol state to
-## getPreviousSection() instead of getCurrentSection() on popSection(), causing
-## a duplicate $x mapping symbol at offset 8 in .text.
-# CHECK: Symbol table '.symtab' contains 5 entries:
+# CHECK: Symbol table '.symtab' contains 4 entries:
# CHECK-NEXT: Num: Value Size Type Bind Vis Ndx Name
# CHECK-NEXT: 0: {{0+}} 0 NOTYPE LOCAL DEFAULT UND {{$}}
# CHECK-RV32-NEXT: 1: 00000000 0 NOTYPE LOCAL DEFAULT [[#TEXT]] $xrv32i2p1{{$}}
@@ -46,6 +43,4 @@
# CHECK-RV32-NEXT: 2: 00000000 0 NOTYPE LOCAL DEFAULT [[#WIBBLE]] $xrv32i2p1{{$}}
# CHECK-RV64-NEXT: 2: {{0+}} 0 NOTYPE LOCAL DEFAULT [[#WIBBLE]] $xrv64i2p1{{$}}
# CHECK-NEXT: 3: {{0+}} 0 NOTYPE LOCAL DEFAULT [[#STARTS_DATA]] $d{{$}}
-# CHECK-RV32-NEXT: 4: 00000008 0 NOTYPE LOCAL DEFAULT [[#TEXT]] $xrv32i2p1{{$}}
-# CHECK-RV64-NEXT: 4: {{0+}}8 0 NOTYPE LOCAL DEFAULT [[#TEXT]] $xrv64i2p1{{$}}
# CHECK-NOT: {{.}}
More information about the llvm-branch-commits
mailing list