Skip to content

Commit c8121b9

Browse files
MaskRayMaskRay
authored
[RISCV] Xqcilb: remove RISCVMCExpr::VK_QC_E_JUMP_PLT and drop @plt parsing
Follow-up to the just landed #135044 . Remove `@plt` parsing (only needed by legacy `call foo@plt`). MCParser's `@` parsing is problematic. Supporting target variations like (`foo+2@plt foo@plt+2 (foo+2)@plt`) involves messy hacks. We should refrain from adding new `@` uses. Remove unneeded `RISCVMCExpr::VK_QC_E_JUMP_PLT` (should only be used when an instruction might have multiple reasonable relocations https://maskray.me/blog/2025-03-16-relocation-generation-in-assemblers). --- GCC's initial initial RISC-V port made a mistake by having both `call foo` (non-PIC) and `call foo@plt` (PIC), likely misled by x86/SystemZ. It was determined that the `@plt` was not needed. Since R_RISCV_CALL had questionable undefined weak semantics in GNU ld (which has been removed then), we kept R_RISCV_CALL_PLT and deprecated R_RISCV_CALL. For RISC-V instructions, we only keep `@` in call/jump for backward compatibility and discourage it for all other instructions. ( There is disagreement about whether `PLT` in `JUMP_PLT` is useful or misleading. MaskRay's opnion: For new branch relocations with procedure call semantics, use `_CALL` and avoid `_PLT` in the relocation name. `_PLT` should only be used in data directives (e.g. R_RISCV_PLT32) to indicate that the address of a function is not significant. ) Pull Request: #135507
1 parent d41e517 commit c8121b9

File tree

7 files changed

+18
-108
lines changed

7 files changed

+18
-108
lines changed

‎llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp

Lines changed: 3 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -587,17 +587,6 @@ struct RISCVOperand final : public MCParsedAsmOperand {
587587
(VK == RISCVMCExpr::VK_CALL || VK == RISCVMCExpr::VK_CALL_PLT);
588588
}
589589

590-
bool isPseudoQCJumpSymbol() const {
591-
int64_t Imm;
592-
// Must be of 'immediate' type but not a constant.
593-
if (!isImm() || evaluateConstantImm(getImm(), Imm))
594-
return false;
595-
596-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
597-
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
598-
VK == RISCVMCExpr::VK_QC_E_JUMP_PLT;
599-
}
600-
601590
bool isPseudoJumpSymbol() const {
602591
int64_t Imm;
603592
// Must be of 'immediate' type but not a constant.
@@ -1598,11 +1587,11 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
15981587
return generateImmOutOfRangeError(Operands, ErrorInfo,
15991588
std::numeric_limits<int32_t>::min(),
16001589
std::numeric_limits<uint32_t>::max());
1601-
case Match_InvalidSImm32Lsb0:
1590+
case Match_InvalidBareSImm32Lsb0:
16021591
return generateImmOutOfRangeError(
16031592
Operands, ErrorInfo, std::numeric_limits<int32_t>::min(),
16041593
std::numeric_limits<int32_t>::max() - 1,
1605-
"operand must be a multiple of 2 bytes in the range ");
1594+
"operand must be a multiple of 2 bytes in the range");
16061595
case Match_InvalidRnumArg: {
16071596
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, 10);
16081597
}
@@ -2151,25 +2140,10 @@ ParseStatus RISCVAsmParser::parsePseudoQCJumpSymbol(OperandVector &Operands) {
21512140

21522141
std::string Identifier(getTok().getIdentifier());
21532142
SMLoc E = getTok().getEndLoc();
2154-
2155-
if (getLexer().peekTok().is(AsmToken::At)) {
2156-
Lex();
2157-
Lex();
2158-
SMLoc PLTLoc = getLoc();
2159-
StringRef PLT;
2160-
E = getTok().getEndLoc();
2161-
if (getParser().parseIdentifier(PLT) || PLT != "plt")
2162-
return Error(PLTLoc,
2163-
"'@plt' is the only valid operand for this instruction");
2164-
} else {
2165-
Lex();
2166-
}
2167-
2168-
RISCVMCExpr::Specifier Kind = RISCVMCExpr::VK_QC_E_JUMP_PLT;
2143+
Lex();
21692144

21702145
MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
21712146
Res = MCSymbolRefExpr::create(Sym, getContext());
2172-
Res = RISCVMCExpr::create(Res, Kind, getContext());
21732147
Operands.push_back(RISCVOperand::createImm(Res, S, E, isRV64()));
21742148
return ParseStatus::Success;
21752149
}

‎llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,6 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
5656
SmallVectorImpl<MCFixup> &Fixups,
5757
const MCSubtargetInfo &STI) const;
5858

59-
void expandQCJump(const MCInst &MI, SmallVectorImpl<char> &CB,
60-
SmallVectorImpl<MCFixup> &Fixups,
61-
const MCSubtargetInfo &STI) const;
62-
6359
void expandTLSDESCCall(const MCInst &MI, SmallVectorImpl<char> &CB,
6460
SmallVectorImpl<MCFixup> &Fixups,
6561
const MCSubtargetInfo &STI) const;
@@ -173,26 +169,6 @@ void RISCVMCCodeEmitter::expandFunctionCall(const MCInst &MI,
173169
support::endian::write(CB, Binary, llvm::endianness::little);
174170
}
175171

176-
void RISCVMCCodeEmitter::expandQCJump(const MCInst &MI,
177-
SmallVectorImpl<char> &CB,
178-
SmallVectorImpl<MCFixup> &Fixups,
179-
const MCSubtargetInfo &STI) const {
180-
MCOperand Func = MI.getOperand(0);
181-
assert(Func.isExpr() && "Expected expression");
182-
183-
auto Opcode =
184-
(MI.getOpcode() == RISCV::PseudoQC_E_J) ? RISCV::QC_E_J : RISCV::QC_E_JAL;
185-
MCInst Jump = MCInstBuilder(Opcode).addExpr(Func.getExpr());
186-
187-
uint64_t Bits = getBinaryCodeForInstr(Jump, Fixups, STI) & 0xffff'ffff'ffffu;
188-
SmallVector<char, 8> Encoding;
189-
support::endian::write(Encoding, Bits, llvm::endianness::little);
190-
assert(Encoding[6] == 0 && Encoding[7] == 0 &&
191-
"Unexpected encoding for 48-bit instruction");
192-
Encoding.truncate(6);
193-
CB.append(Encoding);
194-
}
195-
196172
void RISCVMCCodeEmitter::expandTLSDESCCall(const MCInst &MI,
197173
SmallVectorImpl<char> &CB,
198174
SmallVectorImpl<MCFixup> &Fixups,
@@ -464,11 +440,6 @@ void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI,
464440
expandTLSDESCCall(MI, CB, Fixups, STI);
465441
MCNumEmitted += 1;
466442
return;
467-
case RISCV::PseudoQC_E_J:
468-
case RISCV::PseudoQC_E_JAL:
469-
expandQCJump(MI, CB, Fixups, STI);
470-
MCNumEmitted += 1;
471-
return;
472443
}
473444

474445
switch (Size) {
@@ -685,9 +656,6 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
685656
case RISCVMCExpr::VK_QC_ABS20:
686657
FixupKind = RISCV::fixup_riscv_qc_abs20_u;
687658
break;
688-
case RISCVMCExpr::VK_QC_E_JUMP_PLT:
689-
FixupKind = RISCV::fixup_riscv_qc_e_jump_plt;
690-
break;
691659
}
692660
} else if (Kind == MCExpr::SymbolRef || Kind == MCExpr::Binary) {
693661
// FIXME: Sub kind binary exprs have chance of underflow.
@@ -705,6 +673,8 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
705673
FixupKind = RISCV::fixup_riscv_qc_e_branch;
706674
} else if (MIFrm == RISCVII::InstFormatQC_EAI) {
707675
FixupKind = RISCV::fixup_riscv_qc_e_32;
676+
} else if (MIFrm == RISCVII::InstFormatQC_EJ) {
677+
FixupKind = RISCV::fixup_riscv_qc_e_jump_plt;
708678
}
709679
}
710680

‎llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,7 @@ const RISCVMCExpr *RISCVMCExpr::create(const MCExpr *Expr, Specifier S,
3434

3535
void RISCVMCExpr::printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const {
3636
Specifier S = getSpecifier();
37-
bool HasVariant = ((S != VK_None) && (S != VK_CALL) && (S != VK_CALL_PLT) &&
38-
(S != VK_QC_E_JUMP_PLT));
37+
bool HasVariant = ((S != VK_None) && (S != VK_CALL) && (S != VK_CALL_PLT));
3938

4039
if (HasVariant)
4140
OS << '%' << getSpecifierName(S) << '(';
@@ -168,8 +167,6 @@ StringRef RISCVMCExpr::getSpecifierName(Specifier S) {
168167
return "pltpcrel";
169168
case VK_QC_ABS20:
170169
return "qc.abs20";
171-
case VK_QC_E_JUMP_PLT:
172-
return "qc_e_jump_plt";
173170
}
174171
llvm_unreachable("Invalid ELF symbol kind");
175172
}

‎llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ class RISCVMCExpr : public MCTargetExpr {
4444
VK_TLSDESC_ADD_LO,
4545
VK_TLSDESC_CALL,
4646
VK_QC_ABS20,
47-
VK_QC_E_JUMP_PLT
4847
};
4948

5049
private:

‎llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td

Lines changed: 8 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -139,32 +139,20 @@ def bare_simm32 : RISCVOp<XLenVT> {
139139
}
140140

141141
// A 32-bit signed immediate where the least significant bit is zero.
142-
def simm32_lsb0 : Operand<OtherVT> {
143-
let ParserMatchClass = SImmAsmOperand<32, "Lsb0">;
142+
def bare_simm32_lsb0 : Operand<OtherVT> {
143+
let ParserMatchClass = BareSImmNLsb0AsmOperand<32>;
144144
let PrintMethod = "printBranchOperand";
145145
let EncoderMethod = "getImmOpValueAsr1";
146146
let DecoderMethod = "decodeSImmOperandAndLsl1<32>";
147147
let MCOperandPredicate = [{
148148
int64_t Imm;
149-
if (!MCOp.evaluateAsConstantImm(Imm))
150-
return false;
151-
return isShiftedInt<31, 1>(Imm);
149+
if (MCOp.evaluateAsConstantImm(Imm))
150+
return isShiftedInt<31, 1>(Imm);
151+
return MCOp.isBareSymbolRef();
152152
}];
153153
let OperandType = "OPERAND_PCREL";
154154
}
155155

156-
def PseudoQCJumpSymbol : AsmOperandClass {
157-
let Name = "PseudoQCJumpSymbol";
158-
let RenderMethod = "addImmOperands";
159-
let DiagnosticType = "InvalidPseudoQCJumpSymbol";
160-
let DiagnosticString = "operand must be a valid jump target";
161-
let ParserMethod = "parsePseudoQCJumpSymbol";
162-
}
163-
164-
def pseudo_qc_jump_symbol : Operand<XLenVT> {
165-
let ParserMatchClass = PseudoQCJumpSymbol;
166-
}
167-
168156
//===----------------------------------------------------------------------===//
169157
// Instruction Formats
170158
//===----------------------------------------------------------------------===//
@@ -313,7 +301,7 @@ def InsnQC_EJ : DirectiveInsnQC_EJ<(outs),
313301
uimm3:$func3,
314302
uimm2:$func2,
315303
uimm5:$func5,
316-
simm32_lsb0:$imm31),
304+
bare_simm32_lsb0:$imm31),
317305
"$opcode, $func3, $func2, $func5, $imm31">;
318306
def InsnQC_ES : DirectiveInsnQC_ES<(outs),
319307
(ins uimm7_opcode:$opcode,
@@ -365,7 +353,7 @@ def : InstAlias<".insn_qc.ej $opcode, $func3, $func2, $func5, $imm31",
365353
uimm3:$func3,
366354
uimm2:$func2,
367355
uimm5:$func5,
368-
simm32_lsb0:$imm31)>;
356+
bare_simm32_lsb0:$imm31)>;
369357
def : InstAlias<".insn_qc.es $opcode, $func3, $func2, $rs2, ${imm26}(${rs1})",
370358
(InsnQC_ES uimm7_opcode:$opcode,
371359
uimm3:$func3,
@@ -719,7 +707,7 @@ class QCIRVInstEI<bits<3> funct3, bits<2> funct2, string opcodestr>
719707

720708
let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
721709
class QCIRVInst48EJ<bits<2> func2, string opcodestr>
722-
: RVInst48<(outs), (ins simm32_lsb0:$imm31),
710+
: RVInst48<(outs), (ins bare_simm32_lsb0:$imm31),
723711
opcodestr, "$imm31", [], InstFormatQC_EJ> {
724712
bits<31> imm31;
725713

@@ -1231,16 +1219,6 @@ def PseudoQC_E_SH : PseudoStore<"qc.e.sh">;
12311219
def PseudoQC_E_SW : PseudoStore<"qc.e.sw">;
12321220
} // Predicates = [HasVendorXqcilo, IsRV32]
12331221

1234-
let isCall = 0, isBarrier = 1, isTerminator = 1,
1235-
isCodeGenOnly = 0, hasSideEffects = 0, mayStore = 0, mayLoad = 0 in
1236-
def PseudoQC_E_J : Pseudo<(outs), (ins pseudo_qc_jump_symbol:$func), [],
1237-
"qc.e.j", "$func">;
1238-
1239-
let isCall = 1, Defs = [X1], isCodeGenOnly = 0, hasSideEffects = 0,
1240-
mayStore = 0, mayLoad = 0 in
1241-
def PseudoQC_E_JAL: Pseudo<(outs), (ins pseudo_qc_jump_symbol:$func), [],
1242-
"qc.e.jal", "$func">;
1243-
12441222
//===----------------------------------------------------------------------===//
12451223
// Code Gen Patterns
12461224
//===----------------------------------------------------------------------===//

‎llvm/test/MC/RISCV/xqcilb-invalid.s

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ qc.e.j -2147483649
1313
# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
1414
qc.e.j -2147483648
1515

16+
# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
17+
qc.e.j foo
1618

1719
# CHECK: :[[@LINE+1]]:1: error: too few operands for instruction
1820
qc.e.jal
@@ -23,5 +25,5 @@ qc.e.jal 2147483649
2325
# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
2426
qc.e.jal 2147483640
2527

26-
# CHECK: :[[@LINE+1]]:12: error: '@plt' is the only valid operand for this instruction
27-
qc.e.j foo@rlt
28+
# CHECK: :[[@LINE+1]]:11: error: unexpected token
29+
qc.e.j foo@plt

‎llvm/test/MC/RISCV/xqcilb-relocations.s

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,6 @@ qc.e.j foo
1515
# INSTR: qc.e.j foo
1616
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
1717

18-
qc.e.j foo@plt
19-
# RELOC: R_RISCV_CUSTOM195 foo 0x0
20-
# INSTR: qc.e.j foo
21-
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
22-
23-
qc.e.jal foo@plt
24-
# RELOC: R_RISCV_CUSTOM195 foo 0x0
25-
# INSTR: qc.e.jal foo
26-
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
27-
2818
qc.e.jal foo
2919
# RELOC: R_RISCV_CUSTOM195 foo 0x0
3020
# INSTR: qc.e.jal foo

0 commit comments

Comments
 (0)