From 1b5b5a6df9b18deae38d712e26cb30ceb6872a8c Mon Sep 17 00:00:00 2001 From: Fangrui Song Date: Tue, 20 May 2025 00:48:13 -0700 Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?= =?UTF-8?q?l=20version?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Created using spr 1.3.5-bogner --- llvm/include/llvm/MC/MCAssembler.h | 2 ++ llvm/lib/MC/MCAssembler.cpp | 4 +++ llvm/lib/MC/MCExpr.cpp | 6 ++++ .../Target/RISCV/AsmParser/RISCVAsmParser.cpp | 15 ---------- .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 28 +++++++++++++++---- .../RISCV/MCTargetDesc/RISCVAsmBackend.h | 8 ++++-- .../RISCV/MCTargetDesc/RISCVELFStreamer.cpp | 7 ----- .../CodeGen/RISCV/option-relax-relocation.ll | 10 +++++-- llvm/test/MC/RISCV/option-relax.s | 10 +------ 9 files changed, 48 insertions(+), 42 deletions(-) diff --git a/llvm/include/llvm/MC/MCAssembler.h b/llvm/include/llvm/MC/MCAssembler.h index d463b40cbd142..becd7cf6e8d1b 100644 --- a/llvm/include/llvm/MC/MCAssembler.h +++ b/llvm/include/llvm/MC/MCAssembler.h @@ -64,6 +64,7 @@ class MCAssembler { std::unique_ptr Writer; bool HasLayout = false; + bool HasFinalLayout = false; bool RelaxAll = false; SectionListType Sections; @@ -197,6 +198,7 @@ class MCAssembler { void layout(); bool hasLayout() const { return HasLayout; } + bool hasFinalLayout() const { return HasFinalLayout; } bool getRelaxAll() const { return RelaxAll; } void setRelaxAll(bool Value) { RelaxAll = Value; } diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp index 3a974f3591631..c2e775389735a 100644 --- a/llvm/lib/MC/MCAssembler.cpp +++ b/llvm/lib/MC/MCAssembler.cpp @@ -897,6 +897,10 @@ void MCAssembler::layout() { // example, to set the index fields in the symbol data). getWriter().executePostLayoutBinding(*this); + // Fragments sizes are final. RISC-V style linker relaxation determines + // whether a PC-relative fixup is truly resolved with this flag. + this->HasFinalLayout = true; + // Evaluate and apply the fixups, generating relocation entries as necessary. for (MCSection &Sec : *this) { for (MCFragment &Frag : Sec) { diff --git a/llvm/lib/MC/MCExpr.cpp b/llvm/lib/MC/MCExpr.cpp index 4c159feea48f8..bfde050b5b5ed 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()) { + // A relaxable fragment has indeterminate size before finishLayout. + // Afterwards (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( diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp index f64307babab07..01704e69ddd0b 100644 --- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp +++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp @@ -2820,21 +2820,6 @@ bool RISCVAsmParser::parseOperand(OperandVector &Operands, StringRef Mnemonic) { bool RISCVAsmParser::parseInstruction(ParseInstructionInfo &Info, StringRef Name, SMLoc NameLoc, OperandVector &Operands) { - // Ensure that if the instruction occurs when relaxation is enabled, - // relocations are forced for the file. Ideally this would be done when there - // is enough information to reliably determine if the instruction itself may - // cause relaxations. Unfortunately instruction processing stage occurs in the - // same pass as relocation emission, so it's too late to set a 'sticky bit' - // for the entire file. - if (getSTI().hasFeature(RISCV::FeatureRelax)) { - auto *Assembler = getTargetStreamer().getStreamer().getAssemblerPtr(); - if (Assembler != nullptr) { - RISCVAsmBackend &MAB = - static_cast(Assembler->getBackend()); - MAB.setForceRelocs(); - } - } - // Apply mnemonic aliases because the destination mnemonic may have require // custom operand parsing. The generic tblgen'erated code does this later, at // the start of MatchInstructionImpl(), but that's too late for custom diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index bcab114c0ecf0..aaef249976845 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -104,9 +104,8 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const { return Infos[Kind - FirstTargetFixupKind]; } -// If linker relaxation is enabled, or the relax option had previously been -// enabled, always emit relocations even if the fixup can be resolved. This is -// necessary for correctness as offsets may change during relaxation. +// If linker relaxation is enabled, emit relocation to mark the instruction as +// shrinkable by the linker. bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm, const MCFixup &Fixup, const MCValue &Target, @@ -124,7 +123,7 @@ bool RISCVAsmBackend::shouldForceRelocation(const MCAssembler &Asm, break; } - return STI->hasFeature(RISCV::FeatureRelax) || ForceRelocs; + return STI->hasFeature(RISCV::FeatureRelax); } bool RISCVAsmBackend::fixupNeedsRelaxationAdvanced(const MCAssembler &, @@ -570,6 +569,18 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value, } } +bool RISCVAsmBackend::isPCRelFixupResolved(const MCAssembler &Asm, + const MCSymbol *SymA, + const MCFragment &F) { + if (!PCRelTemp) + PCRelTemp = Asm.getContext().createTempSymbol(); + PCRelTemp->setFragment(const_cast(&F)); + MCValue Res; + MCExpr::evaluateSymbolicAdd(&Asm, false, MCValue::get(SymA), + MCValue::get(nullptr, PCRelTemp), Res); + return !Res.getSubSym(); +} + bool RISCVAsmBackend::evaluateTargetFixup( const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF, const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) { @@ -613,7 +624,9 @@ bool RISCVAsmBackend::evaluateTargetFixup( Value = Asm.getSymbolOffset(SA) + AUIPCTarget.getConstant(); Value -= Asm.getFragmentOffset(*AUIPCDF) + AUIPCFixup->getOffset(); - return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20; + if (AUIPCFixup->getTargetKind() != RISCV::fixup_riscv_pcrel_hi20) + return false; + return isPCRelFixupResolved(Asm, AUIPCTarget.getAddSym(), *AUIPCDF); } bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F, @@ -659,11 +672,14 @@ bool RISCVAsmBackend::addReloc(MCAssembler &Asm, const MCFragment &F, return false; } + if (IsResolved && + (getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel)) + IsResolved = isPCRelFixupResolved(Asm, Target.getAddSym(), F); IsResolved = MCAsmBackend::addReloc(Asm, F, Fixup, Target, FixedValue, IsResolved, STI); // If linker relaxation is enabled and supported by the current relocation, // append a RELAX relocation. - if (Fixup.needsRelax()) { + if (!IsResolved && Fixup.needsRelax()) { auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX); Asm.getWriter().recordRelocation(Asm, &F, FA, MCValue::get(nullptr), FixedValueA); diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h index 874cf654e7eef..e9de9ac642b11 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h @@ -25,16 +25,18 @@ class RISCVAsmBackend : public MCAsmBackend { const MCSubtargetInfo &STI; uint8_t OSABI; bool Is64Bit; - bool ForceRelocs = false; const MCTargetOptions &TargetOptions; + // Temporary symbol used to check whether a PC-relative fixup is resolved. + MCSymbol *PCRelTemp = nullptr; + + bool isPCRelFixupResolved(const MCAssembler &Asm, const MCSymbol *SymA, + const MCFragment &F); public: RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit, const MCTargetOptions &Options); ~RISCVAsmBackend() override = default; - void setForceRelocs() { ForceRelocs = true; } - // Return Size with extra Nop Bytes for alignment directive in code section. bool shouldInsertExtraNopBytesForCodeAlign(const MCAlignFragment &AF, unsigned &Size) override; diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp index 5a81c829e3a89..c654fd2b5cbe0 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp @@ -34,13 +34,6 @@ RISCVTargetELFStreamer::RISCVTargetELFStreamer(MCStreamer &S, 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 - // without knowing a possibly future .option relax. When RISCVAsmParser is used, - // its ParseInstruction may call setForceRelocs as well. - if (STI.hasFeature(RISCV::FeatureRelax)) - static_cast(MAB).setForceRelocs(); } RISCVELFStreamer &RISCVTargetELFStreamer::getStreamer() { diff --git a/llvm/test/CodeGen/RISCV/option-relax-relocation.ll b/llvm/test/CodeGen/RISCV/option-relax-relocation.ll index 3dc5aa64bb368..e4071d16fef67 100644 --- a/llvm/test/CodeGen/RISCV/option-relax-relocation.ll +++ b/llvm/test/CodeGen/RISCV/option-relax-relocation.ll @@ -2,7 +2,7 @@ ;; after linker relaxation. See https://github.com/ClangBuiltLinux/linux/issues/1965 ; RUN: llc -mtriple=riscv64 -mattr=-relax -filetype=obj < %s \ -; RUN: | llvm-objdump -d -r - | FileCheck %s +; RUN: | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,NORELAX ; RUN: llc -mtriple=riscv64 -mattr=+relax -filetype=obj < %s \ ; RUN: | llvm-objdump -d -r - | FileCheck %s --check-prefixes=CHECK,RELAX @@ -12,14 +12,20 @@ ; CHECK-NEXT: R_RISCV_CALL_PLT f ; RELAX-NEXT: R_RISCV_RELAX *ABS* ; CHECK-NEXT: jalr ra +; CHECK-NEXT: j {{.*}} +; CHECK-NEXT: j {{.*}} +; NORELAX-NEXT: li a0, 0x0 +; RELAX-NEXT: R_RISCV_JAL .L0 define dso_local noundef signext i32 @main() local_unnamed_addr #0 { entry: - callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"() #2 + callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"() to label %asm.fallthrough [label %label] asm.fallthrough: ; preds = %entry tail call void @f() + callbr void asm sideeffect ".option push\0A.option norvc\0A.option norelax\0Aj $0\0A.option pop\0A", "!i"() + to label %asm.fallthrough [label %label] br label %label label: ; preds = %asm.fallthrough, %entry diff --git a/llvm/test/MC/RISCV/option-relax.s b/llvm/test/MC/RISCV/option-relax.s index 8a6a929ad0241..7b27293fe727b 100644 --- a/llvm/test/MC/RISCV/option-relax.s +++ b/llvm/test/MC/RISCV/option-relax.s @@ -21,15 +21,11 @@ # CHECK-INST: call foo # CHECK-RELOC: R_RISCV_CALL_PLT foo 0x0 -# CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0 +# CHECK-RELOC-NOT: R_RISCV call foo -# CHECK-RELOC-NEXT: R_RISCV_ADD64 -# CHECK-RELOC-NEXT: R_RISCV_SUB64 .dword .L2-.L1 -# CHECK-RELOC-NEXT: R_RISCV_JAL jal zero, .L1 -# CHECK-RELOC-NEXT: R_RISCV_BRANCH beq s1, s1, .L1 .L2: @@ -41,8 +37,6 @@ beq s1, s1, .L1 # CHECK-RELOC-NEXT: R_RISCV_RELAX - 0x0 call bar -# CHECK-RELOC-NEXT: R_RISCV_ADD64 -# CHECK-RELOC-NEXT: R_RISCV_SUB64 .dword .L2-.L1 # CHECK-RELOC-NEXT: R_RISCV_JAL jal zero, .L1 @@ -57,8 +51,6 @@ beq s1, s1, .L1 # CHECK-RELOC-NOT: R_RISCV_RELAX - 0x0 call baz -# CHECK-RELOC-NEXT: R_RISCV_ADD64 -# CHECK-RELOC-NEXT: R_RISCV_SUB64 .dword .L2-.L1 # CHECK-RELOC-NEXT: R_RISCV_JAL jal zero, .L1