From bb03cdcb441fd68da9d1ebb7d5f39f73667cd39c Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Fri, 23 May 2025 18:44:15 -0700 Subject: RISCV: Remove shouldForceRelocation and unneeded relocations Follow-up to #140494 `shouldForceRelocation` is conservative and produces redundant relocations. For example, RISCVAsmBackend::ForceRelocs (introduced to support mixed relax/norelax code) leads to redundant relocations in the following example adapted from #77436 ``` .option norelax j label // For assembly input, RISCVAsmParser::ParseInstruction sets ForceRelocs (https://reviews.llvm.org/D46423). // For direct object emission, RISCVELFStreamer sets ForceRelocs (#77436) .option relax call foo // linker-relaxable .option norelax j label // redundant relocation due to ForceRelocs .option relax label: ``` Root problem: The `isSymbolRefDifferenceFullyResolvedImpl` condition in MCAssembler::evaluateFixup does not check whether two locations are separated by a fragment whose size can be indeterminate due to linker instruction (e.g. MCDataFragment with relaxation, or MCAlignFragment due to indeterminate start offst). This patch * Updates the fragment walk code in `attemptToFoldSymbolOffsetDifference` to treat MCRelaxableFragment (for --riscv-asm-relax-branches) as fixed size after finishLayout. * Adds a condition in `addReloc` to complement `isSymbolRefDifferenceFullyResolvedImpl`. * Removes the no longer needed `shouldForceRelocation`. This fragment walk code path handles nicely handles mixed relax/norelax case from https://discourse.llvm.org/t/possible-problem-related-to-subtarget-usage/75283 and allows us to remove `MCSubtargetInfo` argument (#73721) as a follow-up. This fragment walk code should be avoided in the absence of linker-relaxable fragments within the current section. Adjust two bolt/test/RISCV tests (#141310) Pull Request: https://github.com/llvm/llvm-project/pull/140692 --- llvm/lib/MC/MCExpr.cpp | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'llvm/lib/MC/MCExpr.cpp') diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp index 4c159fe..765f1a2 100644 --- a/llvm/lib/MC/MCExpr.cpp +++ b/llvm/lib/MC/MCExpr.cpp @@ -390,6 +390,12 @@ static void attemptToFoldSymbolOffsetDifference(const MCAssembler *Asm, unsigned Count; if (DF) { Displacement += DF->getContents().size(); + } else if (auto *RF = dyn_cast(FI); + RF && Asm->hasFinalLayout()) { + // Before finishLayout, a relaxable fragment's size is indeterminate. + // After layout, during relocation generation, it can be treated as a + // data fragment. + Displacement += RF->getContents().size(); } else if (auto *AF = dyn_cast(FI); AF && Layout && AF->hasEmitNops() && !Asm->getBackend().shouldInsertExtraNopBytesForCodeAlign( -- cgit v1.1