Skip to content

[RISCV] Vendor Relocations for Xqci extension #135400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
//===----------------------------------------------------------------------===//

#include "RISCVAsmBackend.h"
#include "RISCVFixupKinds.h"
#include "RISCVMCExpr.h"
#include "llvm/ADT/APInt.h"
#include "llvm/MC/MCAsmInfo.h"
Expand Down Expand Up @@ -611,6 +612,46 @@ bool RISCVAsmBackend::evaluateTargetFixup(const MCFixup &Fixup,
isPCRelFixupResolved(AUIPCTarget.getAddSym(), *AUIPCDF);
}

void RISCVAsmBackend::maybeAddVendorReloc(const MCFragment &F,
const MCFixup &Fixup) {
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:
VendorIdentifier = "QUALCOMM";
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.
MCContext &Ctx = Asm->getContext();
MCSymbol *VendorSymbol = Ctx.createLocalSymbol(VendorIdentifier);
auto [It, Inserted] =
VendorSymbols.try_emplace(VendorIdentifier, VendorSymbol);

if (Inserted) {
// Setup the just-created symbol
VendorSymbol->setVariableValue(MCConstantExpr::create(0, Ctx));
Asm->registerSymbol(*VendorSymbol);
} else {
// Fetch the existing symbol
VendorSymbol = It->getValue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the objdump -t CHECK lines need some -NOT pattern to check that we don't generate redundat local symbols.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

MCFixup VendorFixup =
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);
}

bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup,
const MCValue &Target, uint64_t &FixedValue,
bool IsResolved) {
Expand Down Expand Up @@ -660,7 +701,14 @@ bool RISCVAsmBackend::addReloc(const MCFragment &F, const MCFixup &Fixup,
if (IsResolved &&
(getFixupKindInfo(Fixup.getKind()).Flags & MCFixupKindInfo::FKF_IsPCRel))
IsResolved = isPCRelFixupResolved(Target.getAddSym(), F);
IsResolved = MCAsmBackend::addReloc(F, Fixup, Target, FixedValue, IsResolved);

if (!IsResolved) {
// Some Fixups require a vendor relocation, record it (directly) before we
// add the relocation.
maybeAddVendorReloc(F, Fixup);

Asm->getWriter().recordRelocation(F, Fixup, Target, FixedValue);
}

if (Fixup.isLinkerRelaxable()) {
auto FA = MCFixup::create(Fixup.getOffset(), nullptr, ELF::R_RISCV_RELAX);
Expand Down
5 changes: 5 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -31,6 +32,8 @@ class RISCVAsmBackend : public MCAsmBackend {

bool isPCRelFixupResolved(const MCSymbol *SymA, const MCFragment &F);

StringMap<MCSymbol *> VendorSymbols;

public:
RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options);
Expand All @@ -50,6 +53,8 @@ class RISCVAsmBackend : public MCAsmBackend {
bool addReloc(const MCFragment &, const MCFixup &, const MCValue &,
uint64_t &FixedValue, bool IsResolved) override;

void maybeAddVendorReloc(const MCFragment &, const MCFixup &);

void applyFixup(const MCFragment &, const MCFixup &, const MCValue &Target,
MutableArrayRef<char> Data, uint64_t Value,
bool IsResolved) override;
Expand Down
35 changes: 35 additions & 0 deletions llvm/test/MC/RISCV/vendor-symbol.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# 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-NOT: QUALCOMM
# CHECK: 00000000 0 NOTYPE LOCAL DEFAULT ABS QUALCOMM
# CHECK-NOT: QUALCOMM
# CHECK: 00000006 0 NOTYPE GLOBAL DEFAULT 2 QUALCOMM
# CHECK-NOT: QUALCOMM
103 changes: 74 additions & 29 deletions llvm/test/MC/RISCV/xqcibi-relocations.s
Original file line number Diff line number Diff line change
@@ -1,38 +1,83 @@
# 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.B<op>I 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>:
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 <this_section>
# OBJ-NEXT: R_RISCV_BRANCH undef{{$}}
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-NEXT: qc.e.bgeui s0, 0x14, 0x4 <this_section+0x4>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM193 undef{{$}}
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-NEXT: qc.bnei t2, 0xb, 0x28 <same_section>
qc.bnei t2, 11, same_section

.section .t2
# ASM: qc.e.bgeui s1, 21, same_section
# OBJ-NEXT: qc.e.bgeui s1, 0x15, 0x28 <same_section>
qc.e.bgeui s1, 21, same_section

.bar:
ret

# ASM: qc.bnei t2, 12, same_section_extern
# OBJ-NEXT: qc.bnei t2, 0xc, 0x14 <this_section+0x14>
# OBJ-NEXT: R_RISCV_BRANCH same_section_extern{{$}}
qc.bnei t2, 12, same_section_extern

# ASM: qc.e.bgeui s1, 22, same_section_extern
# OBJ-NEXT: qc.e.bgeui s1, 0x16, 0x18 <this_section+0x18>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM193 same_section_extern{{$}}
qc.e.bgeui s1, 22, same_section_extern


# ASM: qc.bnei t3, 13, other_section
# OBJ-NEXT: qc.bnei t3, 0xd, 0x1e <this_section+0x1e>
# OBJ-NEXT: R_RISCV_BRANCH other_section{{$}}
qc.bnei t3, 13, other_section

# ASM: qc.e.bgeui s2, 23, other_section
# OBJ-NEXT: qc.e.bgeui s2, 0x17, 0x22 <this_section+0x22>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM193 other_section{{$}}
qc.e.bgeui s2, 23, other_section


# ASM-LABEL: same_section:
# OBJ-LABEL: <same_section>:
same_section:
nop

# ASM-LABEL: same_section_extern:
# OBJ-LABEL: <same_section_extern>:
.global same_section_extern
same_section_extern:
nop


.section .text.second, "ax", @progbits

# ASM-LABEL: other_section:
# OBJ-LABEL: <other_section>:
other_section:
nop
104 changes: 71 additions & 33 deletions llvm/test/MC/RISCV/xqcilb-relocations.s
Original file line number Diff line number Diff line change
@@ -1,46 +1,84 @@
# 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These can be resolved within the same section and the destination symbol is local?

Otherwise I think there will be a relocation as well. (We also emit a relocation when the destination is a gnu ifunc symbol, which is an edge case that we don't necessarily describe).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think these always need a relocation. I forgot these are to the PLT, was just focussed on them being pc-relative.

I don't think I found the LLVM code to always emit the relocation (that I expect for call_plt), i will look over it all again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added tests to show that for non-local symbols, we still get the relocation, but for local symbols we don't (which matches when call is resolved directly to the symbol in the assembler)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to edit one of the tests to define QUALCOMM: and use llvm-objdump -t to check the symbol table. There will be two QUALCOMM local symbols, which is fine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree a test for this is needed. I'll add it as a separate test, for clarity.

## (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>:
this_section:

# ASM: qc.e.j undef
# OBJ: qc.e.j 0x0 <this_section>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}}
qc.e.j undef
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the absence of .option exact, qc.e.j undef will be converted to a shorter-range jal, is that intended?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'll upload a separate patch to fix this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in #142702


# ASM: qc.e.jal undef
# OBJ-NEXT: qc.e.jal 0x6 <this_section+0x6>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 undef{{$}}
qc.e.jal undef


# ASM: qc.e.j same_section
# OBJ-NEXT: qc.e.j 0x30 <same_section>
qc.e.j same_section

# ASM: qc.e.jal same_section
# OBJ-NEXT: qc.e.jal 0x30 <same_section>
qc.e.jal same_section

# ASM: qc.e.j same_section_extern
# OBJ-NEXT: qc.e.j 0x18 <this_section+0x18>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 same_section_extern{{$}}
qc.e.j same_section_extern

qc.e.jal foo
# RELOC: R_RISCV_CUSTOM195 foo 0x0
# INSTR: qc.e.jal foo
# ASM: qc.e.jal same_section_extern
# OBJ-NEXT: qc.e.jal 0x1e <this_section+0x1e>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 same_section_extern{{$}}
qc.e.jal same_section_extern

# 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

qc.e.jal .bar
# RELOC: R_RISCV_CUSTOM195 .bar 0x0
# INSTR: qc.e.jal .bar
# ASM: qc.e.j other_section
# OBJ-NEXT: qc.e.j 0x24 <this_section+0x24>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}}
qc.e.j other_section

# Check that jumps to a defined symbol are handled correctly
qc.e.j .L1
# INSTR:qc.e.j .L1
# ASM: qc.e.jal other_section
# OBJ-NEXT: qc.e.jal 0x2a <this_section+0x2a>
# OBJ-NEXT: R_RISCV_VENDOR QUALCOMM{{$}}
# OBJ-NEXT: R_RISCV_CUSTOM195 other_section{{$}}
qc.e.jal other_section

qc.e.jal .L1
# INSTR:qc.e.jal .L1

.option noexact
# ASM-LABEL: same_section:
# OBJ-LABEL: <same_section>:
same_section:
nop

.L1:
ret
# ASM-LABEL: same_section_extern:
# OBJ-LABEL: <same_section_extern>:
.global same_section_extern
same_section_extern:
nop

.section .t2
.section .text.other, "ax", @progbits

.bar:
ret
# ASM-LABEL: other_section:
# OBJ-LABEL: <other_section>:
other_section:
nop
Loading