[llvm] c8121b9 - [RISCV] Xqcilb: remove RISCVMCExpr::VK_QC_E_JUMP_PLT and drop `@plt` parsing
via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 15 09:10:07 PDT 2025
Author: Fangrui Song
Date: 2025-04-15T09:10:04-07:00
New Revision: c8121b99a99fe1785add732aa062039b7c5fdd32
URL: https://github.com/llvm/llvm-project/commit/c8121b99a99fe1785add732aa062039b7c5fdd32
DIFF: https://github.com/llvm/llvm-project/commit/c8121b99a99fe1785add732aa062039b7c5fdd32.diff
LOG: [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 at plt`). MCParser's `@` parsing is problematic.
Supporting target variations like (`foo+2 at plt foo at 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 at 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: https://github.com/llvm/llvm-project/pull/135507
Added:
Modified:
llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
llvm/test/MC/RISCV/xqcilb-invalid.s
llvm/test/MC/RISCV/xqcilb-relocations.s
Removed:
################################################################################
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index 2f8e002f291db..013944787ff2d 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -587,17 +587,6 @@ struct RISCVOperand final : public MCParsedAsmOperand {
(VK == RISCVMCExpr::VK_CALL || VK == RISCVMCExpr::VK_CALL_PLT);
}
- bool isPseudoQCJumpSymbol() const {
- int64_t Imm;
- // Must be of 'immediate' type but not a constant.
- if (!isImm() || evaluateConstantImm(getImm(), Imm))
- return false;
-
- RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
- return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
- VK == RISCVMCExpr::VK_QC_E_JUMP_PLT;
- }
-
bool isPseudoJumpSymbol() const {
int64_t Imm;
// Must be of 'immediate' type but not a constant.
@@ -1598,11 +1587,11 @@ bool RISCVAsmParser::matchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
return generateImmOutOfRangeError(Operands, ErrorInfo,
std::numeric_limits<int32_t>::min(),
std::numeric_limits<uint32_t>::max());
- case Match_InvalidSImm32Lsb0:
+ case Match_InvalidBareSImm32Lsb0:
return generateImmOutOfRangeError(
Operands, ErrorInfo, std::numeric_limits<int32_t>::min(),
std::numeric_limits<int32_t>::max() - 1,
- "operand must be a multiple of 2 bytes in the range ");
+ "operand must be a multiple of 2 bytes in the range");
case Match_InvalidRnumArg: {
return generateImmOutOfRangeError(Operands, ErrorInfo, 0, 10);
}
@@ -2151,25 +2140,10 @@ ParseStatus RISCVAsmParser::parsePseudoQCJumpSymbol(OperandVector &Operands) {
std::string Identifier(getTok().getIdentifier());
SMLoc E = getTok().getEndLoc();
-
- if (getLexer().peekTok().is(AsmToken::At)) {
- Lex();
- Lex();
- SMLoc PLTLoc = getLoc();
- StringRef PLT;
- E = getTok().getEndLoc();
- if (getParser().parseIdentifier(PLT) || PLT != "plt")
- return Error(PLTLoc,
- "'@plt' is the only valid operand for this instruction");
- } else {
- Lex();
- }
-
- RISCVMCExpr::Specifier Kind = RISCVMCExpr::VK_QC_E_JUMP_PLT;
+ Lex();
MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
Res = MCSymbolRefExpr::create(Sym, getContext());
- Res = RISCVMCExpr::create(Res, Kind, getContext());
Operands.push_back(RISCVOperand::createImm(Res, S, E, isRV64()));
return ParseStatus::Success;
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
index f324907d49fd9..6283f1d120aaa 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
@@ -56,10 +56,6 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
- void expandQCJump(const MCInst &MI, SmallVectorImpl<char> &CB,
- SmallVectorImpl<MCFixup> &Fixups,
- const MCSubtargetInfo &STI) const;
-
void expandTLSDESCCall(const MCInst &MI, SmallVectorImpl<char> &CB,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;
@@ -173,26 +169,6 @@ void RISCVMCCodeEmitter::expandFunctionCall(const MCInst &MI,
support::endian::write(CB, Binary, llvm::endianness::little);
}
-void RISCVMCCodeEmitter::expandQCJump(const MCInst &MI,
- SmallVectorImpl<char> &CB,
- SmallVectorImpl<MCFixup> &Fixups,
- const MCSubtargetInfo &STI) const {
- MCOperand Func = MI.getOperand(0);
- assert(Func.isExpr() && "Expected expression");
-
- auto Opcode =
- (MI.getOpcode() == RISCV::PseudoQC_E_J) ? RISCV::QC_E_J : RISCV::QC_E_JAL;
- MCInst Jump = MCInstBuilder(Opcode).addExpr(Func.getExpr());
-
- uint64_t Bits = getBinaryCodeForInstr(Jump, Fixups, STI) & 0xffff'ffff'ffffu;
- SmallVector<char, 8> Encoding;
- support::endian::write(Encoding, Bits, llvm::endianness::little);
- assert(Encoding[6] == 0 && Encoding[7] == 0 &&
- "Unexpected encoding for 48-bit instruction");
- Encoding.truncate(6);
- CB.append(Encoding);
-}
-
void RISCVMCCodeEmitter::expandTLSDESCCall(const MCInst &MI,
SmallVectorImpl<char> &CB,
SmallVectorImpl<MCFixup> &Fixups,
@@ -464,11 +440,6 @@ void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI,
expandTLSDESCCall(MI, CB, Fixups, STI);
MCNumEmitted += 1;
return;
- case RISCV::PseudoQC_E_J:
- case RISCV::PseudoQC_E_JAL:
- expandQCJump(MI, CB, Fixups, STI);
- MCNumEmitted += 1;
- return;
}
switch (Size) {
@@ -685,9 +656,6 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
case RISCVMCExpr::VK_QC_ABS20:
FixupKind = RISCV::fixup_riscv_qc_abs20_u;
break;
- case RISCVMCExpr::VK_QC_E_JUMP_PLT:
- FixupKind = RISCV::fixup_riscv_qc_e_jump_plt;
- break;
}
} else if (Kind == MCExpr::SymbolRef || Kind == MCExpr::Binary) {
// FIXME: Sub kind binary exprs have chance of underflow.
@@ -705,6 +673,8 @@ uint64_t RISCVMCCodeEmitter::getImmOpValue(const MCInst &MI, unsigned OpNo,
FixupKind = RISCV::fixup_riscv_qc_e_branch;
} else if (MIFrm == RISCVII::InstFormatQC_EAI) {
FixupKind = RISCV::fixup_riscv_qc_e_32;
+ } else if (MIFrm == RISCVII::InstFormatQC_EJ) {
+ FixupKind = RISCV::fixup_riscv_qc_e_jump_plt;
}
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
index 99f72620f97ed..d6650e156c8b3 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.cpp
@@ -34,8 +34,7 @@ const RISCVMCExpr *RISCVMCExpr::create(const MCExpr *Expr, Specifier S,
void RISCVMCExpr::printImpl(raw_ostream &OS, const MCAsmInfo *MAI) const {
Specifier S = getSpecifier();
- bool HasVariant = ((S != VK_None) && (S != VK_CALL) && (S != VK_CALL_PLT) &&
- (S != VK_QC_E_JUMP_PLT));
+ bool HasVariant = ((S != VK_None) && (S != VK_CALL) && (S != VK_CALL_PLT));
if (HasVariant)
OS << '%' << getSpecifierName(S) << '(';
@@ -168,8 +167,6 @@ StringRef RISCVMCExpr::getSpecifierName(Specifier S) {
return "pltpcrel";
case VK_QC_ABS20:
return "qc.abs20";
- case VK_QC_E_JUMP_PLT:
- return "qc_e_jump_plt";
}
llvm_unreachable("Invalid ELF symbol kind");
}
diff --git a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
index d60879d34dc17..e0aa7ff244521 100644
--- a/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
+++ b/llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCExpr.h
@@ -44,7 +44,6 @@ class RISCVMCExpr : public MCTargetExpr {
VK_TLSDESC_ADD_LO,
VK_TLSDESC_CALL,
VK_QC_ABS20,
- VK_QC_E_JUMP_PLT
};
private:
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
index 4ac17c8283866..8eaa5e394a91c 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoXqci.td
@@ -139,32 +139,20 @@ def bare_simm32 : RISCVOp<XLenVT> {
}
// A 32-bit signed immediate where the least significant bit is zero.
-def simm32_lsb0 : Operand<OtherVT> {
- let ParserMatchClass = SImmAsmOperand<32, "Lsb0">;
+def bare_simm32_lsb0 : Operand<OtherVT> {
+ let ParserMatchClass = BareSImmNLsb0AsmOperand<32>;
let PrintMethod = "printBranchOperand";
let EncoderMethod = "getImmOpValueAsr1";
let DecoderMethod = "decodeSImmOperandAndLsl1<32>";
let MCOperandPredicate = [{
int64_t Imm;
- if (!MCOp.evaluateAsConstantImm(Imm))
- return false;
- return isShiftedInt<31, 1>(Imm);
+ if (MCOp.evaluateAsConstantImm(Imm))
+ return isShiftedInt<31, 1>(Imm);
+ return MCOp.isBareSymbolRef();
}];
let OperandType = "OPERAND_PCREL";
}
-def PseudoQCJumpSymbol : AsmOperandClass {
- let Name = "PseudoQCJumpSymbol";
- let RenderMethod = "addImmOperands";
- let DiagnosticType = "InvalidPseudoQCJumpSymbol";
- let DiagnosticString = "operand must be a valid jump target";
- let ParserMethod = "parsePseudoQCJumpSymbol";
-}
-
-def pseudo_qc_jump_symbol : Operand<XLenVT> {
- let ParserMatchClass = PseudoQCJumpSymbol;
-}
-
//===----------------------------------------------------------------------===//
// Instruction Formats
//===----------------------------------------------------------------------===//
@@ -313,7 +301,7 @@ def InsnQC_EJ : DirectiveInsnQC_EJ<(outs),
uimm3:$func3,
uimm2:$func2,
uimm5:$func5,
- simm32_lsb0:$imm31),
+ bare_simm32_lsb0:$imm31),
"$opcode, $func3, $func2, $func5, $imm31">;
def InsnQC_ES : DirectiveInsnQC_ES<(outs),
(ins uimm7_opcode:$opcode,
@@ -365,7 +353,7 @@ def : InstAlias<".insn_qc.ej $opcode, $func3, $func2, $func5, $imm31",
uimm3:$func3,
uimm2:$func2,
uimm5:$func5,
- simm32_lsb0:$imm31)>;
+ bare_simm32_lsb0:$imm31)>;
def : InstAlias<".insn_qc.es $opcode, $func3, $func2, $rs2, ${imm26}(${rs1})",
(InsnQC_ES uimm7_opcode:$opcode,
uimm3:$func3,
@@ -719,7 +707,7 @@ class QCIRVInstEI<bits<3> funct3, bits<2> funct2, string opcodestr>
let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
class QCIRVInst48EJ<bits<2> func2, string opcodestr>
- : RVInst48<(outs), (ins simm32_lsb0:$imm31),
+ : RVInst48<(outs), (ins bare_simm32_lsb0:$imm31),
opcodestr, "$imm31", [], InstFormatQC_EJ> {
bits<31> imm31;
@@ -1231,16 +1219,6 @@ def PseudoQC_E_SH : PseudoStore<"qc.e.sh">;
def PseudoQC_E_SW : PseudoStore<"qc.e.sw">;
} // Predicates = [HasVendorXqcilo, IsRV32]
-let isCall = 0, isBarrier = 1, isTerminator = 1,
- isCodeGenOnly = 0, hasSideEffects = 0, mayStore = 0, mayLoad = 0 in
-def PseudoQC_E_J : Pseudo<(outs), (ins pseudo_qc_jump_symbol:$func), [],
- "qc.e.j", "$func">;
-
-let isCall = 1, Defs = [X1], isCodeGenOnly = 0, hasSideEffects = 0,
- mayStore = 0, mayLoad = 0 in
-def PseudoQC_E_JAL: Pseudo<(outs), (ins pseudo_qc_jump_symbol:$func), [],
- "qc.e.jal", "$func">;
-
//===----------------------------------------------------------------------===//
// Code Gen Patterns
//===----------------------------------------------------------------------===//
diff --git a/llvm/test/MC/RISCV/xqcilb-invalid.s b/llvm/test/MC/RISCV/xqcilb-invalid.s
index 10d456c8ac0aa..1c584da890dd3 100644
--- a/llvm/test/MC/RISCV/xqcilb-invalid.s
+++ b/llvm/test/MC/RISCV/xqcilb-invalid.s
@@ -13,6 +13,8 @@ qc.e.j -2147483649
# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
qc.e.j -2147483648
+# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
+qc.e.j foo
# CHECK: :[[@LINE+1]]:1: error: too few operands for instruction
qc.e.jal
@@ -23,5 +25,5 @@ qc.e.jal 2147483649
# CHECK-MINUS: :[[@LINE+1]]:1: error: instruction requires the following: 'Xqcilb' (Qualcomm uC Long Branch Extension)
qc.e.jal 2147483640
-# CHECK: :[[@LINE+1]]:12: error: '@plt' is the only valid operand for this instruction
-qc.e.j foo at rlt
+# CHECK: :[[@LINE+1]]:11: error: unexpected token
+qc.e.j foo at plt
diff --git a/llvm/test/MC/RISCV/xqcilb-relocations.s b/llvm/test/MC/RISCV/xqcilb-relocations.s
index a475cde3f6bfd..92e543d94f6b6 100644
--- a/llvm/test/MC/RISCV/xqcilb-relocations.s
+++ b/llvm/test/MC/RISCV/xqcilb-relocations.s
@@ -15,16 +15,6 @@ qc.e.j foo
# INSTR: qc.e.j foo
# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
-qc.e.j foo at plt
-# RELOC: R_RISCV_CUSTOM195 foo 0x0
-# INSTR: qc.e.j foo
-# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
-
-qc.e.jal foo at plt
-# RELOC: R_RISCV_CUSTOM195 foo 0x0
-# INSTR: qc.e.jal foo
-# FIXUP: fixup A - offset: 0, value: foo, kind: fixup_riscv_qc_e_jump_plt
-
qc.e.jal foo
# RELOC: R_RISCV_CUSTOM195 foo 0x0
# INSTR: qc.e.jal foo
More information about the llvm-commits
mailing list