[llvm] e6fed06 - [RISCV] Make linker-relaxable instructions terminate MCDataFragment
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 29 09:40:01 PDT 2023
Author: Fangrui Song
Date: 2023-06-29T09:39:57-07:00
New Revision: e6fed06335f4a05aac900380c975b2daf8cbb934
URL: https://github.com/llvm/llvm-project/commit/e6fed06335f4a05aac900380c975b2daf8cbb934
DIFF: https://github.com/llvm/llvm-project/commit/e6fed06335f4a05aac900380c975b2daf8cbb934.diff
LOG: [RISCV] Make linker-relaxable instructions terminate MCDataFragment
`MCExpr::evaluateAsAbsolute` has a longstanding bug. When the MCAssembler is
non-null and the MCAsmLayout is null, it may incorrectly fold A-B even if A and
B are separated by a linker-relaxable instruction. This behavior can suppress
some ADD/SUB relocations and lead to wrong results if the linker performs
relaxation.
To fix the bug, ensure that linker-relaxable instructions only appear at the end
of an MCDataFragment, thereby making them terminate the fragment. When computing
A-B, suppress folding if A and B are separated by a linker-relaxable
instruction.
* `.subsection` now correctly give errors for non-foldable expressions.
* gen-dwarf.s will pass even if we add back the .debug_line or .eh_frame/.debug_frame code from D150004
* This will fix suppressed relocation when we add R_RISCV_SET_ULEB128/R_RISCV_SUB_ULEB128.
In the future, we should investigate the desired behavior for
`MCExpr::evaluateAsAbsolute` when both MCAssembler and MCAsmLayout are non-null.
(Note: MCRelaxableFragment is only for assembler-relaxation. If we ever need
linker-relaxable MCRelaxableFragment, we would need to adjust RISCVMCExpr.cpp
(D58943/D73211).)
Depends on D153096
Differential Revision: https://reviews.llvm.org/D153097
Added:
Modified:
llvm/include/llvm/MC/MCAsmBackend.h
llvm/include/llvm/MC/MCFragment.h
llvm/lib/MC/MCAsmBackend.cpp
llvm/lib/MC/MCELFStreamer.cpp
llvm/lib/MC/MCExpr.cpp
llvm/lib/MC/MCObjectStreamer.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
llvm/test/MC/ELF/RISCV/subsection.s
Removed:
################################################################################
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index c219991f65a59b..11bb612c4f30e3 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -41,7 +41,8 @@ class raw_ostream;
/// Generic interface to target specific assembler backends.
class MCAsmBackend {
protected: // Can only create subclasses.
- MCAsmBackend(support::endianness Endian);
+ MCAsmBackend(support::endianness Endian,
+ unsigned RelaxFixupKind = MaxFixupKind);
public:
MCAsmBackend(const MCAsmBackend &) = delete;
@@ -50,6 +51,9 @@ class MCAsmBackend {
const support::endianness Endian;
+ /// Fixup kind used for linker relaxation. Currently only used by RISC-V.
+ const unsigned RelaxFixupKind;
+
/// Return true if this target might automatically pad instructions and thus
/// need to emit padding enable/disable directives around sensative code.
virtual bool allowAutoPadding() const { return false; }
diff --git a/llvm/include/llvm/MC/MCFragment.h b/llvm/include/llvm/MC/MCFragment.h
index 0c7e97368e2134..7be4792a452198 100644
--- a/llvm/include/llvm/MC/MCFragment.h
+++ b/llvm/include/llvm/MC/MCFragment.h
@@ -75,6 +75,7 @@ class MCFragment : public ilist_node_with_parent<MCFragment, MCSection> {
protected:
bool HasInstructions;
+ bool LinkerRelaxable = false;
MCFragment(FragmentType Kind, bool HasInstructions,
MCSection *Parent = nullptr);
@@ -246,6 +247,9 @@ class MCDataFragment : public MCEncodedFragmentWithFixups<32, 4> {
static bool classof(const MCFragment *F) {
return F->getKind() == MCFragment::FT_Data;
}
+
+ bool isLinkerRelaxable() const { return LinkerRelaxable; }
+ void setLinkerRelaxable() { LinkerRelaxable = true; }
};
/// This is a compact (memory-size-wise) fragment for holding an encoded
diff --git a/llvm/lib/MC/MCAsmBackend.cpp b/llvm/lib/MC/MCAsmBackend.cpp
index 4fe61d5f265fb7..64bbc63719c7f7 100644
--- a/llvm/lib/MC/MCAsmBackend.cpp
+++ b/llvm/lib/MC/MCAsmBackend.cpp
@@ -22,7 +22,8 @@
using namespace llvm;
-MCAsmBackend::MCAsmBackend(support::endianness Endian) : Endian(Endian) {}
+MCAsmBackend::MCAsmBackend(support::endianness Endian, unsigned RelaxFixupKind)
+ : Endian(Endian), RelaxFixupKind(RelaxFixupKind) {}
MCAsmBackend::~MCAsmBackend() = default;
diff --git a/llvm/lib/MC/MCELFStreamer.cpp b/llvm/lib/MC/MCELFStreamer.cpp
index 303eb16011d17f..653ff4e9435a52 100644
--- a/llvm/lib/MC/MCELFStreamer.cpp
+++ b/llvm/lib/MC/MCELFStreamer.cpp
@@ -628,6 +628,9 @@ void MCELFStreamer::emitInstToData(const MCInst &Inst,
}
DF->setHasInstructions(STI);
+ if (!Fixups.empty() && Fixups.back().getTargetKind() ==
+ getAssembler().getBackend().RelaxFixupKind)
+ DF->setLinkerRelaxable();
DF->getContents().append(Code.begin(), Code.end());
if (Assembler.isBundlingEnabled() && Assembler.getRelaxAll()) {
diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp
index a8d3fef9a8d2f5..a73ae524b8e0ab 100644
--- a/llvm/lib/MC/MCExpr.cpp
+++ b/llvm/lib/MC/MCExpr.cpp
@@ -659,7 +659,6 @@ static void AttemptToFoldSymbolOffsetDifference(
// Try to find a constant displacement from FA to FB, add the displacement
// between the offset in FA of SA and the offset in FB of SB.
- int64_t Displacement = SA.getOffset() - SB.getOffset();
bool Reverse = false;
if (FA == FB) {
Reverse = SA.getOffset() < SB.getOffset();
@@ -667,14 +666,31 @@ static void AttemptToFoldSymbolOffsetDifference(
Reverse = std::find_if(std::next(FA->getIterator()), SecA.end(),
[&](auto &I) { return &I == FB; }) != SecA.end();
}
+
+ uint64_t SAOffset = SA.getOffset(), SBOffset = SB.getOffset();
+ int64_t Displacement = SA.getOffset() - SB.getOffset();
if (Reverse) {
std::swap(FA, FB);
+ std::swap(SAOffset, SBOffset);
Displacement *= -1;
}
[[maybe_unused]] bool Found = false;
+ // Track whether B is before a relaxable instruction and whether A is after
+ // a relaxable instruction. If SA and SB are separated by a linker-relaxable
+ // instruction, the
diff erence cannot be resolved as it may be changed by
+ // the linker.
+ bool BBeforeRelax = false, AAfterRelax = false;
for (auto FI = FB->getIterator(), FE = SecA.end(); FI != FE; ++FI) {
auto DF = dyn_cast<MCDataFragment>(FI);
+ if (DF && DF->isLinkerRelaxable()) {
+ if (&*FI != FB || SBOffset != DF->getContents().size())
+ BBeforeRelax = true;
+ if (&*FI != FA || SAOffset == DF->getContents().size())
+ AAfterRelax = true;
+ if (BBeforeRelax && AAfterRelax)
+ return;
+ }
if (&*FI == FA) {
Found = true;
break;
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index 9398eb85618aa4..3cf7b4359cabab 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -211,6 +211,11 @@ static bool canReuseDataFragment(const MCDataFragment &F,
const MCSubtargetInfo *STI) {
if (!F.hasInstructions())
return true;
+ // Do not add data after a linker-relaxable instruction. The
diff erence
+ // between a new label and a label at or before the linker-relaxable
+ // instruction cannot be resolved at assemble-time.
+ if (F.isLinkerRelaxable())
+ return false;
// When bundling is enabled, we don't want to add data to a fragment that
// already has instructions (see MCELFStreamer::emitInstToData for details)
if (Assembler.isBundlingEnabled())
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
index b5670b6214c213..7fff5e1060c706 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
@@ -31,8 +31,8 @@ class RISCVAsmBackend : public MCAsmBackend {
public:
RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options)
- : MCAsmBackend(support::little), STI(STI), OSABI(OSABI), Is64Bit(Is64Bit),
- TargetOptions(Options) {
+ : MCAsmBackend(support::little, RISCV::fixup_riscv_relax), STI(STI),
+ OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) {
RISCVFeatures::validate(STI.getTargetTriple(), STI.getFeatureBits());
}
~RISCVAsmBackend() override = default;
diff --git a/llvm/test/MC/ELF/RISCV/subsection.s b/llvm/test/MC/ELF/RISCV/subsection.s
index 144b9cae8aaf8f..ed6524c246ff7f 100644
--- a/llvm/test/MC/ELF/RISCV/subsection.s
+++ b/llvm/test/MC/ELF/RISCV/subsection.s
@@ -1,5 +1,5 @@
-# RUN: not llvm-mc -filetype=obj -triple=riscv64 -mattr=-relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
-# RUN: not llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefix=ERR --implicit-check-not=error:
+# RUN: not llvm-mc -filetype=obj -triple=riscv64 -mattr=-relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefixes=ERR,NORELAX --implicit-check-not=error:
+# RUN: not llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax %s -o /dev/null 2>&1 | FileCheck %s --check-prefixes=ERR,RELAX --implicit-check-not=error:
a:
nop
@@ -11,21 +11,28 @@ d:
.data
## Positive subsection numbers
+## With relaxation, report an error as c-b is not an assemble-time constant.
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection c-b
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection d-b
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection c-a
.subsection b-a
.subsection d-c
## Negative subsection numbers
-# ERR: :[[#@LINE+1]]:14: error: subsection number -8 is not within [0,2147483647]
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -8 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection b-c
-# ERR: :[[#@LINE+1]]:14: error: subsection number -12 is not within [0,2147483647]
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -12 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection b-d
-# ERR: :[[#@LINE+1]]:14: error: subsection number -12 is not within [0,2147483647]
+# NORELAX: :[[#@LINE+2]]:14: error: subsection number -12 is not within [0,2147483647]
+# RELAX: :[[#@LINE+1]]:14: error: cannot evaluate subsection number
.subsection a-c
-# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
+# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
.subsection a-b
-# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
+# ERR: :[[#@LINE+1]]:14: error: subsection number -4 is not within [0,2147483647]
.subsection c-d
More information about the llvm-commits
mailing list