From df04e3169e6dc7f38376f713892e4040b5fa7052 Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Fri, 16 May 2025 16:23:49 -0700 Subject: [PATCH 1/7] [RISCV] Vendor Relocations for Xqci extension This patch implements vendor relocation support for RISC-V, starting with the Xqci extensions. This is done by re-organising `RISCVAsmBackend::evaluateTargetFixup`, to also handle emitting the vendor relocation when the fixup is unresolved. An extra parameter `bool RecordReloc` is added to `evaluateTargetFixup`, so that the relocation is only emitted when it should be, as `evaluateTargetFixup` works the same way. This adds tests showing resolveable and unresolvable symbols for the xqci fixups (some of which are absolute and some of which are pc-relative), including tests when relaxation is enabled which forces relocations that might otherwise not be needed. Co-authored-by: Sudharsan Veeravalli --- llvm/include/llvm/MC/MCAsmBackend.h | 4 +- llvm/lib/MC/MCAssembler.cpp | 4 +- .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 171 ++++++++++++++---- .../RISCV/MCTargetDesc/RISCVAsmBackend.h | 15 +- llvm/test/MC/RISCV/xqcibi-relocations-relax.s | 38 ++++ llvm/test/MC/RISCV/xqcibi-relocations.s | 90 ++++++--- llvm/test/MC/RISCV/xqcilb-relocations-relax.s | 38 ++++ llvm/test/MC/RISCV/xqcilb-relocations.s | 92 ++++++---- llvm/test/MC/RISCV/xqcili-relocations-relax.s | 51 ++++++ llvm/test/MC/RISCV/xqcili-relocations.s | 106 +++++++---- 10 files changed, 472 insertions(+), 137 deletions(-) create mode 100644 llvm/test/MC/RISCV/xqcibi-relocations-relax.s create mode 100644 llvm/test/MC/RISCV/xqcilb-relocations-relax.s create mode 100644 llvm/test/MC/RISCV/xqcili-relocations-relax.s diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h index 691988ae1f1df..02a001b2a6ab1 100644 --- a/llvm/include/llvm/MC/MCAsmBackend.h +++ b/llvm/include/llvm/MC/MCAsmBackend.h @@ -111,8 +111,8 @@ class MCAsmBackend { virtual bool evaluateTargetFixup(const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF, const MCValue &Target, - const MCSubtargetInfo *STI, - uint64_t &Value) { + const MCSubtargetInfo *STI, uint64_t &Value, + bool RecordReloc) { llvm_unreachable("Need to implement hook if target has custom fixups"); } diff --git a/llvm/lib/MC/MCAssembler.cpp b/llvm/lib/MC/MCAssembler.cpp index 73c3a3a00b5c6..2c046f6ad4bee 100644 --- a/llvm/lib/MC/MCAssembler.cpp +++ b/llvm/lib/MC/MCAssembler.cpp @@ -158,8 +158,8 @@ bool MCAssembler::evaluateFixup(const MCFixup &Fixup, const MCFragment *DF, bool IsResolved = false; unsigned FixupFlags = getBackend().getFixupKindInfo(Fixup.getKind()).Flags; if (FixupFlags & MCFixupKindInfo::FKF_IsTarget) { - IsResolved = - getBackend().evaluateTargetFixup(*this, Fixup, DF, Target, STI, Value); + IsResolved = getBackend().evaluateTargetFixup(*this, Fixup, DF, Target, STI, + Value, RecordReloc); } else { const MCSymbol *Add = Target.getAddSym(); const MCSymbol *Sub = Target.getSubSym(); diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index d8cfeb07e52b6..d45fb8efcc8f1 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "RISCVAsmBackend.h" +#include "RISCVFixupKinds.h" #include "RISCVMCExpr.h" #include "llvm/ADT/APInt.h" #include "llvm/MC/MCAsmInfo.h" @@ -37,7 +38,7 @@ static cl::opt ULEB128Reloc( RISCVAsmBackend::RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit, const MCTargetOptions &Options) : MCAsmBackend(llvm::endianness::little, ELF::R_RISCV_RELAX), STI(STI), - OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options) { + OSABI(OSABI), Is64Bit(Is64Bit), TargetOptions(Options), VendorSymbols() { RISCVFeatures::validate(STI.getTargetTriple(), STI.getFeatureBits()); } @@ -84,10 +85,12 @@ MCFixupKindInfo RISCVAsmBackend::getFixupKindInfo(MCFixupKind Kind) const { {"fixup_riscv_call", 0, 64, MCFixupKindInfo::FKF_IsPCRel}, {"fixup_riscv_call_plt", 0, 64, MCFixupKindInfo::FKF_IsPCRel}, - {"fixup_riscv_qc_e_branch", 0, 48, MCFixupKindInfo::FKF_IsPCRel}, - {"fixup_riscv_qc_e_32", 16, 32, 0}, - {"fixup_riscv_qc_abs20_u", 12, 20, 0}, - {"fixup_riscv_qc_e_jump_plt", 0, 48, MCFixupKindInfo::FKF_IsPCRel}, + {"fixup_riscv_qc_e_branch", 0, 48, + MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget}, + {"fixup_riscv_qc_e_32", 16, 32, MCFixupKindInfo::FKF_IsTarget}, + {"fixup_riscv_qc_abs20_u", 12, 20, MCFixupKindInfo::FKF_IsTarget}, + {"fixup_riscv_qc_e_jump_plt", 0, 48, + MCFixupKindInfo::FKF_IsPCRel | MCFixupKindInfo::FKF_IsTarget}, }; static_assert((std::size(Infos)) == RISCV::NumTargetFixupKinds, "Not all fixup kinds added to Infos array"); @@ -571,37 +574,10 @@ static uint64_t adjustFixupValue(const MCFixup &Fixup, uint64_t Value, } } -bool RISCVAsmBackend::evaluateTargetFixup( - const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF, - const MCValue &Target, const MCSubtargetInfo *STI, uint64_t &Value) { - const MCFixup *AUIPCFixup; - const MCFragment *AUIPCDF; - MCValue AUIPCTarget; - switch (Fixup.getTargetKind()) { - default: - llvm_unreachable("Unexpected fixup kind!"); - case RISCV::fixup_riscv_pcrel_hi20: - AUIPCFixup = &Fixup; - AUIPCDF = DF; - AUIPCTarget = Target; - break; - case RISCV::fixup_riscv_pcrel_lo12_i: - case RISCV::fixup_riscv_pcrel_lo12_s: { - AUIPCFixup = cast(Fixup.getValue())->getPCRelHiFixup(&AUIPCDF); - if (!AUIPCFixup) { - Asm.getContext().reportError(Fixup.getLoc(), - "could not find corresponding %pcrel_hi"); - return true; - } - - // MCAssembler::evaluateFixup will emit an error for this case when it sees - // the %pcrel_hi, so don't duplicate it when also seeing the %pcrel_lo. - const MCExpr *AUIPCExpr = AUIPCFixup->getValue(); - if (!AUIPCExpr->evaluateAsRelocatable(AUIPCTarget, &Asm)) - return true; - break; - } - } +static bool evaluateAUIPCFixup(const MCAssembler &Asm, + const MCFixup *AUIPCFixup, + const MCFragment *AUIPCDF, MCValue AUIPCTarget, + uint64_t &Value) { if (!AUIPCTarget.getAddSym()) return false; @@ -622,6 +598,129 @@ bool RISCVAsmBackend::evaluateTargetFixup( return AUIPCFixup->getTargetKind() == RISCV::fixup_riscv_pcrel_hi20; } +StringRef +RISCVAsmBackend::getVendorSymbolNameForRelocation(unsigned FixupKind) const { + switch (FixupKind) { + case RISCV::fixup_riscv_qc_e_branch: + case RISCV::fixup_riscv_qc_abs20_u: + case RISCV::fixup_riscv_qc_e_32: + case RISCV::fixup_riscv_qc_e_jump_plt: + return "QUALCOMM"; + } + + return ""; +} + +bool RISCVAsmBackend::evaluateVendorFixup(const MCAssembler &Asm, + const MCFixup &Fixup, + const MCFragment *DF, + const MCValue &Target, + const MCSubtargetInfo *STI, + uint64_t &Value, bool RecordReloc) { + // This is a copy of the target-independent branch of + // MCAssembler::evaluateFixup + bool IsResolved = false; + const MCSymbol *Add = Target.getAddSym(); + const MCSymbol *Sub = Target.getSubSym(); + Value = Target.getConstant(); + if (Add && Add->isDefined()) + Value += Asm.getSymbolOffset(*Add); + if (Sub && Sub->isDefined()) + Value -= Asm.getSymbolOffset(*Sub); + + unsigned FixupFlags = getFixupKindInfo(Fixup.getKind()).Flags; + if (FixupFlags & MCFixupKindInfo::FKF_IsPCRel) { + Value -= Asm.getFragmentOffset(*DF) + Fixup.getOffset(); + + if (Add && !Sub && !Add->isUndefined() && !Add->isAbsolute()) { + IsResolved = Asm.getWriter().isSymbolRefDifferenceFullyResolvedImpl( + Asm, *Add, *DF, false, true); + } + } else { + IsResolved = Target.isAbsolute(); + } + // End copy of MCAssembler::evaluateFixup + + // If we failed to resolve, or we need to force relocations (relaxations), then + // record a vendor relocation too. + if ((!IsResolved || shouldForceRelocation(Asm, Fixup, Target, STI)) && + RecordReloc) { + // Here are the additions to emit a vendor relocation for fixups that need + // them. + MCContext &Ctx = Asm.getContext(); + + StringRef VendorIdentifier = + getVendorSymbolNameForRelocation(Fixup.getTargetKind()); + + auto It = VendorSymbols.find(VendorIdentifier); + if (It == VendorSymbols.end()) { + auto *VendorSymbol = + cast(Ctx.createLocalSymbol(VendorIdentifier)); + + // Vendor Symbols are Absolute, Local, NOTYPE. + VendorSymbol->setType(ELF::STT_NOTYPE); + VendorSymbol->setBinding(ELF::STB_LOCAL); + VendorSymbol->setVariableValue(MCConstantExpr::create(0, Ctx)); + const_cast(Asm).registerSymbol(*VendorSymbol); + + It = VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol).first; + } + + MCSymbolELF *VendorSymbol = It->getValue(); + const MCExpr *VendorExpr = MCSymbolRefExpr::create(VendorSymbol, Ctx); + MCFixup VendorFixup = + MCFixup::create(Fixup.getOffset(), VendorExpr, + FirstLiteralRelocationKind + ELF::R_RISCV_VENDOR); + // Explicitly create MCValue so that the absolute symbol is not evaluated to + // a constant. + MCValue VendorTarget = MCValue::get(VendorSymbol); + uint64_t VendorValue; + Asm.getWriter().recordRelocation(const_cast(Asm), DF, + VendorFixup, VendorTarget, VendorValue); + } + + return IsResolved; +} + +bool RISCVAsmBackend::evaluateTargetFixup(const MCAssembler &Asm, + const MCFixup &Fixup, + const MCFragment *DF, + const MCValue &Target, + const MCSubtargetInfo *STI, + uint64_t &Value, bool RecordReloc) { + switch (Fixup.getTargetKind()) { + case RISCV::fixup_riscv_pcrel_hi20: + return evaluateAUIPCFixup(Asm, &Fixup, DF, Target, Value); + case RISCV::fixup_riscv_pcrel_lo12_i: + case RISCV::fixup_riscv_pcrel_lo12_s: { + const MCFragment *AUIPCDF; + const MCFixup *AUIPCFixup = + cast(Fixup.getValue())->getPCRelHiFixup(&AUIPCDF); + if (!AUIPCFixup) { + Asm.getContext().reportError(Fixup.getLoc(), + "could not find corresponding %pcrel_hi"); + return true; + } + + // MCAssembler::evaluateFixup will emit an error for this case when it sees + // the %pcrel_hi, so don't duplicate it when also seeing the %pcrel_lo. + const MCExpr *AUIPCExpr = AUIPCFixup->getValue(); + MCValue AUIPCTarget; + if (!AUIPCExpr->evaluateAsRelocatable(AUIPCTarget, &Asm)) + return true; + + return evaluateAUIPCFixup(Asm, AUIPCFixup, AUIPCDF, AUIPCTarget, Value); + } + case RISCV::fixup_riscv_qc_e_branch: + case RISCV::fixup_riscv_qc_abs20_u: + case RISCV::fixup_riscv_qc_e_32: + case RISCV::fixup_riscv_qc_e_jump_plt: + return evaluateVendorFixup(Asm, Fixup, DF, Target, STI, Value, RecordReloc); + } + + llvm_unreachable("Unexpected target fixup kind!"); +} + bool RISCVAsmBackend::handleAddSubRelocations(const MCAssembler &Asm, const MCFragment &F, const MCFixup &Fixup, diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h index 5db55ad0b8567..9ef2adb06a1ef 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h @@ -12,6 +12,7 @@ #include "MCTargetDesc/RISCVBaseInfo.h" #include "MCTargetDesc/RISCVFixupKinds.h" #include "MCTargetDesc/RISCVMCTargetDesc.h" +#include "llvm/ADT/StringMap.h" #include "llvm/MC/MCAsmBackend.h" #include "llvm/MC/MCFixupKindInfo.h" #include "llvm/MC/MCSubtargetInfo.h" @@ -20,6 +21,7 @@ namespace llvm { class MCAssembler; class MCObjectTargetWriter; class raw_ostream; +class MCSymbolELF; class RISCVAsmBackend : public MCAsmBackend { const MCSubtargetInfo &STI; @@ -28,6 +30,8 @@ class RISCVAsmBackend : public MCAsmBackend { bool ForceRelocs = false; const MCTargetOptions &TargetOptions; + StringMap VendorSymbols; + public: RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit, const MCTargetOptions &Options); @@ -45,8 +49,15 @@ class RISCVAsmBackend : public MCAsmBackend { bool evaluateTargetFixup(const MCAssembler &Asm, const MCFixup &Fixup, const MCFragment *DF, const MCValue &Target, - const MCSubtargetInfo *STI, - uint64_t &Value) override; + const MCSubtargetInfo *STI, uint64_t &Value, + bool RecordReloc) override; + + bool evaluateVendorFixup(const MCAssembler &Asm, const MCFixup &Fixup, + const MCFragment *DF, const MCValue &Target, + const MCSubtargetInfo *STI, uint64_t &Value, + bool RecordReloc); + + StringRef getVendorSymbolNameForRelocation(unsigned TargetFixupKind) const; bool handleAddSubRelocations(const MCAssembler &Asm, const MCFragment &F, const MCFixup &Fixup, const MCValue &Target, diff --git a/llvm/test/MC/RISCV/xqcibi-relocations-relax.s b/llvm/test/MC/RISCV/xqcibi-relocations-relax.s new file mode 100644 index 0000000000000..ae1398499cba4 --- /dev/null +++ b/llvm/test/MC/RISCV/xqcibi-relocations-relax.s @@ -0,0 +1,38 @@ +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcibi - \ +# RUN: | FileCheck -check-prefix=OBJ %s + +## This test checks that we emit the right relocations for Xqcibi +## relative branches, when relaxations are enabled. These require a relocation. +## The QC.E.BI instructions also require a vendor relocation. + +# These are required to turn off autocompression, but to re-enable +# linker relaxation. +.option exact +.option relax + +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: + +# ASM: qc.bnei t2, 11, same_section +# OBJ: qc.bnei t2, 0xb, 0x0 +# OBJ-NEXT: R_RISCV_BRANCH same_section{{$}} +# OBJ-NOT: R_RISCV +qc.bnei t2, 11, same_section + +# ASM: qc.e.bgeui s1, 21, same_section +# OBJ: qc.e.bgeui s1, 0x15, 0x4 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM193 same_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.bgeui s1, 21, same_section + +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: +nop + diff --git a/llvm/test/MC/RISCV/xqcibi-relocations.s b/llvm/test/MC/RISCV/xqcibi-relocations.s index 054a890a4b5e5..6d2349e43eabe 100644 --- a/llvm/test/MC/RISCV/xqcibi-relocations.s +++ b/llvm/test/MC/RISCV/xqcibi-relocations.s @@ -1,38 +1,70 @@ -# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s -show-encoding \ -# RUN: | FileCheck -check-prefix=INSTR %s -# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-xqcibi %s -o %t.o -# RUN: llvm-readobj -r %t.o | FileCheck -check-prefix=RELOC %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcibi - \ +# RUN: | FileCheck -check-prefix=OBJ %s -# Check prefixes: -# RELOC - Check the relocation in the object. -# INSTR - Check the instruction is handled properly by the ASMPrinter. +## This test checks that we emit the right relocations for Xqcibi +## relative branches. These can be resolved within the same section +## (when relaxations are disabled) but otherwise require a relocation. +## The QC.E.BI instructions also require a vendor relocation. -.text +# This is required so that the conditional branches requiring relocations +# are not converted into inverted branches with long jumps by the assembler. +.option exact -# Since foo is undefined, this will be relaxed to (qc.beqi + jal) -qc.bnei x6, 10, foo -# RELOC: R_RISCV_JAL foo 0x0 -# INSTR: qc.bnei t1, 10, foo +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: -# Since foo is undefined, this will be relaxed to (qc.e.bltui + jal) -qc.e.bgeui x8, 12, foo -# RELOC: R_RISCV_JAL foo 0x0 -# INSTR: qc.e.bgeui s0, 12, foo +# ASM: qc.bnei t1, 10, undef +# OBJ: qc.bnei t1, 0xa, 0x0 +# OBJ-NEXT: R_RISCV_BRANCH undef{{$}} +# OBJ-NOT: R_RISCV +qc.bnei t1, 10, undef -# Check that a label in a different section is handled similar to an undefined -# symbol and gets relaxed to (qc.e.bgeui + jal) -qc.e.bltui x4, 9, .bar -# RELOC: R_RISCV_JAL .bar 0x0 -# INSTR: qc.e.bltui tp, 9, .bar +# ASM: qc.e.bgeui s0, 20, undef +# OBJ: qc.e.bgeui s0, 0x14, 0x4 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM193 undef{{$}} +# OBJ-NOT: R_RISCV +qc.e.bgeui s0, 20, undef -# Check that branches to a defined symbol are handled correctly -qc.e.beqi x7, 8, .L1 -# INSTR: qc.e.beqi t2, 8, .L1 -.L1: - ret +# ASM: qc.bnei t2, 11, same_section +# OBJ: qc.bnei t2, 0xb, 0x1e +# OBJ-NOT: R_RISCV +qc.bnei t2, 11, same_section -.section .t2 +# ASM: qc.e.bgeui s1, 21, same_section +# OBJ: qc.e.bgeui s1, 0x15, 0x1e +# OBJ-NOT: R_RISCV +qc.e.bgeui s1, 21, same_section -.bar: - ret + +# ASM: qc.bnei t3, 12, other_section +# OBJ: qc.bnei t3, 0xc, 0x14 +# OBJ-NEXT: R_RISCV_BRANCH other_section{{$}} +# OBJ-NOT: R_RISCV +qc.bnei t3, 12, other_section + +# ASM: qc.e.bgeui s2, 22, other_section +# OBJ: qc.e.bgeui s2, 0x16, 0x18 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM193 other_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.bgeui s2, 22, other_section + + +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: + nop + +.section .text.second, "ax", @progbits + +# ASM-LABEL: other_section: +# OBJ-LABEL: : +other_section: + nop diff --git a/llvm/test/MC/RISCV/xqcilb-relocations-relax.s b/llvm/test/MC/RISCV/xqcilb-relocations-relax.s new file mode 100644 index 0000000000000..7de048d78c8e1 --- /dev/null +++ b/llvm/test/MC/RISCV/xqcilb-relocations-relax.s @@ -0,0 +1,38 @@ +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcilb %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcilb %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcilb - \ +# RUN: | FileCheck -check-prefix=OBJ %s + +## This test checks that we emit the right relocations for Xqcilb +## relative jumps when relocations are enabled. These require a +## vendor-specific relocation pair. + +# These are required to turn off autocompression, but to re-enable +# linker relaxation. +.option exact +.option relax + +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: + +# ASM: qc.e.j same_section +# OBJ: qc.e.j 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 same_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.j same_section + +# ASM: qc.e.jal same_section +# OBJ: qc.e.jal 0x6 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 same_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.jal same_section + +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: +nop diff --git a/llvm/test/MC/RISCV/xqcilb-relocations.s b/llvm/test/MC/RISCV/xqcilb-relocations.s index 0a5b788a110da..97cdca3d53944 100644 --- a/llvm/test/MC/RISCV/xqcilb-relocations.s +++ b/llvm/test/MC/RISCV/xqcilb-relocations.s @@ -1,46 +1,72 @@ -# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcilb %s -show-encoding \ -# RUN: | FileCheck -check-prefix=INSTR %s -# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-xqcilb %s -o %t.o -# RUN: llvm-readobj -r %t.o | FileCheck -check-prefix=RELOC %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcilb %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcilb %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcilb - \ +# RUN: | FileCheck -check-prefix=OBJ %s -# Check prefixes: -# RELOC - Check the relocation in the object. -# INSTR - Check the instruction is handled properly by the ASMPrinter. - -.text +## This test checks that we emit the right relocations for Xqcilb +## relative jumps. These can be resolved within the same section +## (when relaxations are disabled) but otherwise require a +## vendor-specific relocation pair. +# This is required so that the conditional jumps are not compressed +# by the assembler .option exact -qc.e.j foo -# RELOC: R_RISCV_CUSTOM195 foo 0x0 -# INSTR: qc.e.j foo +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: + +# ASM: qc.e.j undef +# OBJ: qc.e.j 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}} +# OBJ-NOT: R_RISCV +qc.e.j undef + +# ASM: qc.e.jal undef +# OBJ: qc.e.jal 0x6 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}} +# OBJ-NOT: R_RISCV +qc.e.jal undef + -qc.e.jal foo -# RELOC: R_RISCV_CUSTOM195 foo 0x0 -# INSTR: qc.e.jal foo +# ASM: qc.e.j same_section +# OBJ: qc.e.j 0x24 +# OBJ-NOT: R_RISCV +qc.e.j same_section -# Check that a label in a different section is handled similar to an undefined symbol -qc.e.j .bar -# RELOC: R_RISCV_CUSTOM195 .bar 0x0 -# INSTR: qc.e.j .bar +# ASM: qc.e.jal same_section +# OBJ: qc.e.jal 0x24 +# OBJ-NOT: R_RISCV +qc.e.jal same_section -qc.e.jal .bar -# RELOC: R_RISCV_CUSTOM195 .bar 0x0 -# INSTR: qc.e.jal .bar -# Check that jumps to a defined symbol are handled correctly -qc.e.j .L1 -# INSTR:qc.e.j .L1 +# ASM: qc.e.j other_section +# OBJ: qc.e.j 0x18 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.j other_section -qc.e.jal .L1 -# INSTR:qc.e.jal .L1 +# ASM: qc.e.jal other_section +# OBJ: qc.e.jal 0x1e +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.jal other_section -.option noexact -.L1: - ret +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: + nop -.section .t2 +.section .text.other, "ax", @progbits -.bar: - ret +# ASM-LABEL: other_section: +# OBJ-LABEL: : +other_section: + nop diff --git a/llvm/test/MC/RISCV/xqcili-relocations-relax.s b/llvm/test/MC/RISCV/xqcili-relocations-relax.s new file mode 100644 index 0000000000000..ade3d4fb661be --- /dev/null +++ b/llvm/test/MC/RISCV/xqcili-relocations-relax.s @@ -0,0 +1,51 @@ +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcili %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcili %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcili - \ +# RUN: | FileCheck -check-prefix=OBJ %s + +## This test checks that we emit the right relocations for Xqcili +## immediates, when relaxations are enabled. These don't require +## relocations, but the behaviour is different depending on whether +## there is a target-specific symbol specifier or not (there is +## one for `qc.li`). + +## It might be more obvious if both of these did the same thing, +## either both emitting relocations, or both not emitting relocations. + +# These are required to turn off autocompression, but to re-enable +# linker relaxation. +.option exact +.option relax + +.set abs_symbol, 0x0 + +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: + + +# ASM: qc.li a1, %qc.abs20(0) +# OBJ: qc.li a1, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM192 *ABS*{{$}} +# OBJ-NOT: R_RISCV +qc.li a1, %qc.abs20(abs_symbol) + +# ASM: qc.e.li s1, 0 +# OBJ: qc.e.li s1, 0x0 +# OBJ-NOT: R_RISCV +qc.e.li s1, abs_symbol + +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: + nop + +.section .text.other, "ax", @progbits + +# ASM-LABEL: other_section: +# OBJ-LABEL: : +other_section: + nop diff --git a/llvm/test/MC/RISCV/xqcili-relocations.s b/llvm/test/MC/RISCV/xqcili-relocations.s index 9c709e68334cd..3e7d3487b65f8 100644 --- a/llvm/test/MC/RISCV/xqcili-relocations.s +++ b/llvm/test/MC/RISCV/xqcili-relocations.s @@ -1,46 +1,86 @@ -# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcili %s -show-encoding \ -# RUN: | FileCheck -check-prefix=INSTR %s -# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+experimental-xqcili %s -o %t.o -# RUN: llvm-readobj -r %t.o | FileCheck -check-prefix=RELOC %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcili %s \ +# RUN: | FileCheck -check-prefix=ASM %s +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcili %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-objdump -dr --mattr=+experimental-xqcili - \ +# RUN: | FileCheck -check-prefix=OBJ %s -# Check prefixes: -# RELOC - Check the relocation in the object. -# INSTR - Check the instruction is handled properly by the ASMPrinter. - -.text +## This test checks that we emit the right relocations for Xqcili +## immediates. These always require a relocation pair, unless the +## target is absolute. +# This is required so that the conditional branches requiring relocations +# are not converted into inverted branches with long jumps by the assembler. .option exact -qc.li x4, %qc.abs20(foo) -# RELOC: R_RISCV_CUSTOM192 foo 0x0 -# INSTR: qc.li tp, %qc.abs20(foo) +.set abs_symbol, 0x0 + +# ASM-LABEL: this_section: +# OBJ-LABEL: : +this_section: + +# ASM: qc.li a0, %qc.abs20(undef) +# OBJ: qc.li a0, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM192 undef{{$}} +# OBJ-NOT: R_RISCV +qc.li a0, %qc.abs20(undef) + +# ASM: qc.e.li s0, undef +# OBJ: qc.e.li s0, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM194 undef{{$}} +# OBJ-NOT: R_RISCV +qc.e.li s0, undef + + +# ASM: qc.li a1, %qc.abs20(0) +# OBJ: qc.li a1, 0x0 +# OBJ-NOT: R_RISCV +qc.li a1, %qc.abs20(abs_symbol) -qc.e.li x5, foo -# RELOC: R_RISCV_CUSTOM194 foo 0x0 -# INSTR: qc.e.li t0, foo +# ASM: qc.e.li s1, 0 +# OBJ: qc.e.li s1, 0x0 +# OBJ-NOT: R_RISCV +qc.e.li s1, abs_symbol -# Check that a label in a different section is handled similar to an undefined symbol -qc.li x9, %qc.abs20(.bar) -# RELOC: R_RISCV_CUSTOM192 .bar 0x0 -# INSTR: qc.li s1, %qc.abs20(.bar) -qc.e.li x8, .bar -# RELOC: R_RISCV_CUSTOM194 .bar 0x0 -# INSTR: qc.e.li s0, .bar +# ASM: qc.li a2, %qc.abs20(same_section) +# OBJ: qc.li a2, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM192 same_section{{$}} +# OBJ-NOT: R_RISCV +qc.li a2, %qc.abs20(same_section) -# Check that branches to a defined symbol are handled correctly -qc.li x7, %qc.abs20(.L1) -# INSTR: qc.li t2, %qc.abs20(.L1) +# ASM: qc.e.li s2, same_section +# OBJ: qc.e.li s2, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM194 same_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.li s2, same_section -qc.e.li x6, .L1 -# INSTR: qc.e.li t1, .L1 +# ASM: qc.li a3, %qc.abs20(other_section) +# OBJ: qc.li a3, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM192 other_section{{$}} +# OBJ-NOT: R_RISCV +qc.li a3, %qc.abs20(other_section) -.option noexact +# ASM: qc.e.li s3, other_section +# OBJ: qc.e.li s3, 0x0 +# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} +# OBJ-NEXT: R_RISCV_CUSTOM194 other_section{{$}} +# OBJ-NOT: R_RISCV +qc.e.li s3, other_section -.L1: - ret +# ASM-LABEL: same_section: +# OBJ-LABEL: : +same_section: + nop -.section .t2 +.section .text.other, "ax", @progbits -.bar: - ret +# ASM-LABEL: other_section: +# OBJ-LABEL: : +other_section: + nop From 1e430858f26e0ce6786e31383accb1ab3c00b759 Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Fri, 16 May 2025 18:45:28 -0700 Subject: [PATCH 2/7] clang-format --- llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index d45fb8efcc8f1..87aeacf28b7da 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -641,8 +641,8 @@ bool RISCVAsmBackend::evaluateVendorFixup(const MCAssembler &Asm, } // End copy of MCAssembler::evaluateFixup - // If we failed to resolve, or we need to force relocations (relaxations), then - // record a vendor relocation too. + // If we failed to resolve, or we need to force relocations (relaxations), + // then record a vendor relocation too. if ((!IsResolved || shouldForceRelocation(Asm, Fixup, Target, STI)) && RecordReloc) { // Here are the additions to emit a vendor relocation for fixups that need From 723863fe548e93da5fac051b8cb2b8a81240787d Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Wed, 28 May 2025 11:33:15 -0700 Subject: [PATCH 3/7] Address Review Feedback --- .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 15 +++++++----- llvm/test/MC/RISCV/xqcibi-relocations.s | 22 ++++++----------- llvm/test/MC/RISCV/xqcilb-relocations.s | 24 +++++++------------ llvm/test/MC/RISCV/xqcili-relocations.s | 22 ++++++----------- 4 files changed, 31 insertions(+), 52 deletions(-) diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index aebf67332ab7c..427fd648b7f5a 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -641,8 +641,7 @@ void RISCVAsmBackend::addVendorReloc(const MCFragment &F, const MCFixup &Fixup, MCSymbol *VendorSymbol = It->getValue(); const MCExpr *VendorExpr = MCSymbolRefExpr::create(VendorSymbol, Ctx); MCFixup VendorFixup = - MCFixup::create(Fixup.getOffset(), VendorExpr, - FirstLiteralRelocationKind + ELF::R_RISCV_VENDOR); + MCFixup::create(Fixup.getOffset(), VendorExpr, ELF::R_RISCV_VENDOR); // Explicitly create MCValue so that the absolute symbol is not evaluated to // a constant. MCValue VendorTarget = MCValue::get(VendorSymbol); @@ -700,14 +699,18 @@ bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup, (getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel)) IsResolved = isPCRelFixupResolved(Target.getAddSym(), F); - // Some Fixups require a vendor relocation, record it (directly) before we add - // the relocation. - if (!IsResolved || shouldForceRelocation(Fixup, Target)) { + if (IsResolved && shouldForceRelocation(Fixup, Target)) + IsResolved = false; + + if (!IsResolved) { + // Some Fixups require a vendor relocation, record it (directly) before we + // add the relocation. if (std::optional VendorIdentifier = getVendorIdentifierForFixup(Fixup.getTargetKind())) addVendorReloc(F, Fixup, *VendorIdentifier); + + Asm->getWriter().recordRelocation(F, Fixup, Target, FixedValue); } - IsResolved = MCAsmBackend::addReloc(F, Fixup, Target, FixedValue, IsResolved); if (Fixup.isLinkerRelaxable()) { auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX); diff --git a/llvm/test/MC/RISCV/xqcibi-relocations.s b/llvm/test/MC/RISCV/xqcibi-relocations.s index ee022a060b801..dff0b2b53c31b 100644 --- a/llvm/test/MC/RISCV/xqcibi-relocations.s +++ b/llvm/test/MC/RISCV/xqcibi-relocations.s @@ -21,53 +21,45 @@ this_section: # ASM: qc.bnei t1, 10, undef # OBJ: qc.bnei t1, 0xa, 0x0 # OBJ-NEXT: R_RISCV_BRANCH undef{{$}} -# OBJ-NOT: R_RISCV qc.bnei t1, 10, undef # ASM: qc.e.bgeui s0, 20, undef -# OBJ: qc.e.bgeui s0, 0x14, 0x4 +# OBJ-NEXT: qc.e.bgeui s0, 0x14, 0x4 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM193 undef{{$}} -# OBJ-NOT: R_RISCV qc.e.bgeui s0, 20, undef # ASM: qc.bnei t2, 11, same_section -# OBJ: qc.bnei t2, 0xb, 0x28 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.bnei t2, 0xb, 0x28 qc.bnei t2, 11, same_section # ASM: qc.e.bgeui s1, 21, same_section -# OBJ: qc.e.bgeui s1, 0x15, 0x28 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.e.bgeui s1, 0x15, 0x28 qc.e.bgeui s1, 21, same_section # ASM: qc.bnei t2, 12, same_section_extern -# OBJ: qc.bnei t2, 0xc, 0x14 +# OBJ-NEXT: qc.bnei t2, 0xc, 0x14 # OBJ-NEXT: R_RISCV_BRANCH same_section_extern{{$}} -# OBJ-NOT: R_RISCV qc.bnei t2, 12, same_section_extern # ASM: qc.e.bgeui s1, 22, same_section_extern -# OBJ: qc.e.bgeui s1, 0x16, 0x18 +# OBJ-NEXT: qc.e.bgeui s1, 0x16, 0x18 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM193 same_section_extern{{$}} -# OBJ-NOT: R_RISCV qc.e.bgeui s1, 22, same_section_extern # ASM: qc.bnei t3, 13, other_section -# OBJ: qc.bnei t3, 0xd, 0x1e +# OBJ-NEXT: qc.bnei t3, 0xd, 0x1e # OBJ-NEXT: R_RISCV_BRANCH other_section{{$}} -# OBJ-NOT: R_RISCV qc.bnei t3, 13, other_section # ASM: qc.e.bgeui s2, 23, other_section -# OBJ: qc.e.bgeui s2, 0x17, 0x22 +# OBJ-NEXT: qc.e.bgeui s2, 0x17, 0x22 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM193 other_section{{$}} -# OBJ-NOT: R_RISCV qc.e.bgeui s2, 23, other_section diff --git a/llvm/test/MC/RISCV/xqcilb-relocations.s b/llvm/test/MC/RISCV/xqcilb-relocations.s index b00f835a3dc56..ced7f4fd99f5f 100644 --- a/llvm/test/MC/RISCV/xqcilb-relocations.s +++ b/llvm/test/MC/RISCV/xqcilb-relocations.s @@ -19,57 +19,49 @@ this_section: # ASM: qc.e.j undef -# OBJ: qc.e.j 0x0 +# OBJ: qc.e.j 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}} -# OBJ-NOT: R_RISCV qc.e.j undef # ASM: qc.e.jal undef -# OBJ: qc.e.jal 0x6 +# OBJ-NEXT: qc.e.jal 0x6 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}} -# OBJ-NOT: R_RISCV qc.e.jal undef # ASM: qc.e.j same_section -# OBJ: qc.e.j 0x30 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.e.j 0x30 qc.e.j same_section # ASM: qc.e.jal same_section -# OBJ: qc.e.jal 0x30 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.e.jal 0x30 qc.e.jal same_section # ASM: qc.e.j same_section_extern -# OBJ: qc.e.j 0x18 +# OBJ-NEXT: qc.e.j 0x18 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 same_section_extern{{$}} -# OBJ-NOT: R_RISCV qc.e.j same_section_extern # ASM: qc.e.jal same_section_extern -# OBJ: qc.e.jal 0x1e +# OBJ-NEXT: qc.e.jal 0x1e # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 same_section_extern{{$}} -# OBJ-NOT: R_RISCV qc.e.jal same_section_extern # ASM: qc.e.j other_section -# OBJ: qc.e.j 0x24 +# OBJ-NEXT: qc.e.j 0x24 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}} -# OBJ-NOT: R_RISCV qc.e.j other_section # ASM: qc.e.jal other_section -# OBJ: qc.e.jal 0x2a +# OBJ-NEXT: qc.e.jal 0x2a # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}} -# OBJ-NOT: R_RISCV qc.e.jal other_section diff --git a/llvm/test/MC/RISCV/xqcili-relocations.s b/llvm/test/MC/RISCV/xqcili-relocations.s index 3e7d3487b65f8..ccedd42654a10 100644 --- a/llvm/test/MC/RISCV/xqcili-relocations.s +++ b/llvm/test/MC/RISCV/xqcili-relocations.s @@ -23,54 +23,46 @@ this_section: # OBJ: qc.li a0, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM192 undef{{$}} -# OBJ-NOT: R_RISCV qc.li a0, %qc.abs20(undef) # ASM: qc.e.li s0, undef -# OBJ: qc.e.li s0, 0x0 +# OBJ-NEXT: qc.e.li s0, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM194 undef{{$}} -# OBJ-NOT: R_RISCV qc.e.li s0, undef # ASM: qc.li a1, %qc.abs20(0) -# OBJ: qc.li a1, 0x0 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.li a1, 0x0 qc.li a1, %qc.abs20(abs_symbol) # ASM: qc.e.li s1, 0 -# OBJ: qc.e.li s1, 0x0 -# OBJ-NOT: R_RISCV +# OBJ-NEXT: qc.e.li s1, 0x0 qc.e.li s1, abs_symbol # ASM: qc.li a2, %qc.abs20(same_section) -# OBJ: qc.li a2, 0x0 +# OBJ-NEXT: qc.li a2, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM192 same_section{{$}} -# OBJ-NOT: R_RISCV qc.li a2, %qc.abs20(same_section) # ASM: qc.e.li s2, same_section -# OBJ: qc.e.li s2, 0x0 +# OBJ-NEXT: qc.e.li s2, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM194 same_section{{$}} -# OBJ-NOT: R_RISCV qc.e.li s2, same_section # ASM: qc.li a3, %qc.abs20(other_section) -# OBJ: qc.li a3, 0x0 +# OBJ-NEXT: qc.li a3, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM192 other_section{{$}} -# OBJ-NOT: R_RISCV qc.li a3, %qc.abs20(other_section) # ASM: qc.e.li s3, other_section -# OBJ: qc.e.li s3, 0x0 +# OBJ-NEXT: qc.e.li s3, 0x0 # OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}} # OBJ-NEXT: R_RISCV_CUSTOM194 other_section{{$}} -# OBJ-NOT: R_RISCV qc.e.li s3, other_section # ASM-LABEL: same_section: From 123e3129eb39d0983a94f1a85bfc0f0813ecf9bd Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Wed, 28 May 2025 21:15:30 -0700 Subject: [PATCH 4/7] Address review comments --- llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 3 --- llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h | 3 +-- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index 427fd648b7f5a..d252ffc4dd82b 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -699,9 +699,6 @@ bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup, (getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel)) IsResolved = isPCRelFixupResolved(Target.getAddSym(), F); - if (IsResolved && shouldForceRelocation(Fixup, Target)) - IsResolved = false; - if (!IsResolved) { // Some Fixups require a vendor relocation, record it (directly) before we // add the relocation. diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h index 0f333db0b9006..8de98c13902fa 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h @@ -21,7 +21,6 @@ namespace llvm { class MCAssembler; class MCObjectTargetWriter; class raw_ostream; -class MCSymbolELF; class RISCVAsmBackend : public MCAsmBackend { const MCSubtargetInfo &STI; @@ -52,7 +51,7 @@ class RISCVAsmBackend : public MCAsmBackend { uint64_t &Value) override; std::optional - getVendorIdentifierForFixup(unsigned TargetFixupKind) const; + getVendorIdentifierForFixup(unsigned FixupKind) const; bool addReloc(const MCFragment &, const MCFixup &, const MCValue &, uint64_t &FixedValue, bool IsResolved) override; From d408f176c1c09a3fb64e25729d7b8cd253b36fc3 Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Tue, 3 Jun 2025 12:39:02 -0700 Subject: [PATCH 5/7] Address feedback --- .../RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 45 ++++++++++--------- .../RISCV/MCTargetDesc/RISCVAsmBackend.h | 6 +-- llvm/test/MC/RISCV/vendor-symbol.s | 32 +++++++++++++ 3 files changed, 56 insertions(+), 27 deletions(-) create mode 100644 llvm/test/MC/RISCV/vendor-symbol.s diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index 70ff2f7bba97b..51864cd3b2d4c 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -39,7 +39,7 @@ static cl::opt ULEB128Reloc( RISCVAsmBackend::RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit, const MCTargetOptions &Options) : MCAsmBackend(llvm::endianness::little), STI(STI), OSABI(OSABI), - Is64Bit(Is64Bit), TargetOptions(Options), VendorSymbols() { + Is64Bit(Is64Bit), TargetOptions(Options) { RISCVFeatures::validate(STI.getTargetTriple(), STI.getFeatureBits()); } @@ -612,38 +612,41 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup, isPCRelFixupResolved(AUIPCTarget.getAddSym(), *AUIPCDF); } -std::optional -RISCVAsmBackend::getVendorIdentifierForFixup(unsigned FixupKind) const { - switch (FixupKind) { +void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, const MCFixup &Fixup) { + MCContext &Ctx = Asm->getContext(); + + StringRef VendorIdentifier; + + switch (Fixup.getTargetKind()) { + default: + // No Vendor Relocation Required. + return; case RISCV::fixup_riscv_qc_e_branch: case RISCV::fixup_riscv_qc_abs20_u: case RISCV::fixup_riscv_qc_e_32: case RISCV::fixup_riscv_qc_e_jump_plt: - return "QUALCOMM"; + VendorIdentifier = "QUALCOMM"; + break; } - return std::nullopt; -} - -void RISCVAsmBackend::addVendorReloc(const MCFragment &F, const MCFixup &Fixup, - StringRef VendorIdentifier) { - MCContext &Ctx = Asm->getContext(); + // Create a local symbol for the vendor relocation to reference. It's fine if the symbol has the same name as an existing symbol. + MCSymbol *VendorSymbol = Ctx.createLocalSymbol(VendorIdentifier); + auto [It, Inserted] = VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol); - auto It = VendorSymbols.find(VendorIdentifier); - if (It == VendorSymbols.end()) { - MCSymbol *VendorSymbol = Ctx.createLocalSymbol(VendorIdentifier); + if (Inserted) { + // Setup the just-created symbol VendorSymbol->setVariableValue(MCConstantExpr::create(0, Ctx)); Asm->registerSymbol(*VendorSymbol); - - It = VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol).first; + } else { + // Fetch the existing symbol + VendorSymbol = It->getValue(); } - MCSymbol *VendorSymbol = It->getValue(); const MCExpr *VendorExpr = MCSymbolRefExpr::create(VendorSymbol, Ctx); MCFixup VendorFixup = MCFixup::create(Fixup.getOffset(), VendorExpr, ELF::R_RISCV_VENDOR); - // Explicitly create MCValue so that the absolute symbol is not evaluated to - // a constant. + // Explicitly create MCValue rather than using `VendorExpr->evaluateAsRelocatable` + // so that the absolute symbol is not evaluated to a constant. MCValue VendorTarget = MCValue::get(VendorSymbol); uint64_t VendorValue; Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue); @@ -702,9 +705,7 @@ bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup, if (!IsResolved) { // Some Fixups require a vendor relocation, record it (directly) before we // add the relocation. - if (std::optional VendorIdentifier = - getVendorIdentifierForFixup(Fixup.getTargetKind())) - addVendorReloc(F, Fixup, *VendorIdentifier); + maybeAddVendorReloc(F, Fixup); Asm->getWriter().recordRelocation(F, Fixup, Target, FixedValue); } diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h index 8de98c13902fa..91efd44547509 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h @@ -50,14 +50,10 @@ class RISCVAsmBackend : public MCAsmBackend { bool evaluateTargetFixup(const MCFixup &Fixup, const MCValue &Target, uint64_t &Value) override; - std::optional - getVendorIdentifierForFixup(unsigned FixupKind) const; - bool addReloc(const MCFragment &, const MCFixup &, const MCValue &, uint64_t &FixedValue, bool IsResolved) override; - void addVendorReloc(const MCFragment &, const MCFixup &, - StringRef VendorSymbol); + void maybeAddVendorReloc(const MCFragment &, const MCFixup &); void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target, MutableArrayRef Data, uint64_t Value, diff --git a/llvm/test/MC/RISCV/vendor-symbol.s b/llvm/test/MC/RISCV/vendor-symbol.s new file mode 100644 index 0000000000000..48a47c6fdcf8e --- /dev/null +++ b/llvm/test/MC/RISCV/vendor-symbol.s @@ -0,0 +1,32 @@ +# RUN: llvm-mc -triple riscv32 -mattr=+experimental-xqcibi %s \ +# RUN: -filetype=obj -o - \ +# RUN: | llvm-readelf -sr - \ +# RUN: | FileCheck %s + + +## This checks that the vendor identifier symbols required for vendor +## relocations do not interfere with symbols with identical names that +## are written in assembly. + +.option exact + + qc.e.bgeui s0, 20, QUALCOMM + + .global QUALCOMM +QUALCOMM: + nop + + qc.e.bgeui s0, 20, QUALCOMM + + +# CHECK-LABEL: Relocation section '.rela.text' +## Note the different values for the "Sym. Value" Field +# CHECK: R_RISCV_VENDOR 00000000 QUALCOMM + 0 +# CHECK: R_RISCV_CUSTOM193 00000006 QUALCOMM + 0 +# CHECK: R_RISCV_VENDOR 00000000 QUALCOMM + 0 +# CHECK: R_RISCV_CUSTOM193 00000006 QUALCOMM + 0 + + +# CHECK-LABEL: Symbol table '.symtab' +# CHECK: 00000000 0 NOTYPE LOCAL DEFAULT ABS QUALCOMM +# CHECK: 00000006 0 NOTYPE GLOBAL DEFAULT 2 QUALCOMM From 241ea1bc2e80618d5ffe1f025218f73438ac71d4 Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Tue, 3 Jun 2025 12:44:36 -0700 Subject: [PATCH 6/7] clang-format --- .../Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index 51864cd3b2d4c..cd9bcc4c7bbf3 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -612,7 +612,8 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup, isPCRelFixupResolved(AUIPCTarget.getAddSym(), *AUIPCDF); } -void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, const MCFixup &Fixup) { +void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, + const MCFixup &Fixup) { MCContext &Ctx = Asm->getContext(); StringRef VendorIdentifier; @@ -629,9 +630,11 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, const MCFixup &Fi break; } - // Create a local symbol for the vendor relocation to reference. It's fine if the symbol has the same name as an existing symbol. + // Create a local symbol for the vendor relocation to reference. It's fine if + // the symbol has the same name as an existing symbol. MCSymbol *VendorSymbol = Ctx.createLocalSymbol(VendorIdentifier); - auto [It, Inserted] = VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol); + auto [It, Inserted] = + VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol); if (Inserted) { // Setup the just-created symbol @@ -645,8 +648,9 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, const MCFixup &Fi const MCExpr *VendorExpr = MCSymbolRefExpr::create(VendorSymbol, Ctx); MCFixup VendorFixup = MCFixup::create(Fixup.getOffset(), VendorExpr, ELF::R_RISCV_VENDOR); - // Explicitly create MCValue rather than using `VendorExpr->evaluateAsRelocatable` - // so that the absolute symbol is not evaluated to a constant. + // Explicitly create MCValue rather than using + // `VendorExpr->evaluateAsRelocatable` so that the absolute symbol is not + // evaluated to constant 0. MCValue VendorTarget = MCValue::get(VendorSymbol); uint64_t VendorValue; Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue); From 6b3ba87152d22d91f836bd2c75ef049c8aeb705f Mon Sep 17 00:00:00 2001 From: Sam Elliott Date: Tue, 3 Jun 2025 21:11:26 -0700 Subject: [PATCH 7/7] Address review feedback --- .../Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp | 12 ++++-------- llvm/test/MC/RISCV/vendor-symbol.s | 5 ++++- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp index cd9bcc4c7bbf3..44b1a83c5c17c 100644 --- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp +++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp @@ -614,10 +614,7 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup, void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, const MCFixup &Fixup) { - MCContext &Ctx = Asm->getContext(); - StringRef VendorIdentifier; - switch (Fixup.getTargetKind()) { default: // No Vendor Relocation Required. @@ -632,6 +629,7 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, // Create a local symbol for the vendor relocation to reference. It's fine if // the symbol has the same name as an existing symbol. + MCContext &Ctx = Asm->getContext(); MCSymbol *VendorSymbol = Ctx.createLocalSymbol(VendorIdentifier); auto [It, Inserted] = VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol); @@ -645,12 +643,10 @@ void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F, VendorSymbol = It->getValue(); } - const MCExpr *VendorExpr = MCSymbolRefExpr::create(VendorSymbol, Ctx); MCFixup VendorFixup = - MCFixup::create(Fixup.getOffset(), VendorExpr, ELF::R_RISCV_VENDOR); - // Explicitly create MCValue rather than using - // `VendorExpr->evaluateAsRelocatable` so that the absolute symbol is not - // evaluated to constant 0. + MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_VENDOR); + // Explicitly create MCValue rather than using an MCExpr and evaluating it so + // that the absolute vendor symbol is not evaluated to constant 0. MCValue VendorTarget = MCValue::get(VendorSymbol); uint64_t VendorValue; Asm->getWriter().recordRelocation(F, VendorFixup, VendorTarget, VendorValue); diff --git a/llvm/test/MC/RISCV/vendor-symbol.s b/llvm/test/MC/RISCV/vendor-symbol.s index 48a47c6fdcf8e..7df3a3efeb64e 100644 --- a/llvm/test/MC/RISCV/vendor-symbol.s +++ b/llvm/test/MC/RISCV/vendor-symbol.s @@ -8,7 +8,7 @@ ## relocations do not interfere with symbols with identical names that ## are written in assembly. -.option exact + .option exact qc.e.bgeui s0, 20, QUALCOMM @@ -28,5 +28,8 @@ QUALCOMM: # CHECK-LABEL: Symbol table '.symtab' +# CHECK-NOT: QUALCOMM # CHECK: 00000000 0 NOTYPE LOCAL DEFAULT ABS QUALCOMM +# CHECK-NOT: QUALCOMM # CHECK: 00000006 0 NOTYPE GLOBAL DEFAULT 2 QUALCOMM +# CHECK-NOT: QUALCOMM