Skip to content

Commit 002af0c

Browse files
lenarylenary
committed
[RISCV] Align MCOperandPredicates with AsmParser
Before this change, many MCOperandPredicates accepted bare symbols but there was no cohesive logic as to which did and which didn't, and it did not correspond to where the parser did and did not accept bare symbols. MCOperandPredicates are used by the assembly printer when choosing to whether to use an alias, and by the assembler compression and decompression mechanism. RISC-V uses both of these extensively. Xqci had a bug with a compress pattern for `qc.e.li` (to `c.li`) which was causing crashes because `RISCVMCCodeEmitter::getImmOpValue` could not handle a bare symbol ref when encoding the `c.li` instruction. This patch aligns the MCOperandPredicates with the predicates in the RISC-V assembly parser, denying bare symbol references in places they were accepted for aliases/compression in the past. This is not NFC, as some assembly printing has changed with the `li` alias for `addi`, but this seems very minor. The crash when assembling `qc.e.li a0, undef` is now fixed.
1 parent c8f25f7 commit 002af0c

File tree

13 files changed

+98
-36
lines changed

13 files changed

+98
-36
lines changed

‎llvm/include/llvm/MC/MCExpr.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ class MCExpr {
4646
Target ///< Target specific expression.
4747
};
4848

49+
/// The type of a Specifier (used in MCSymbolRefExpr and MCSpecifierExpr).
50+
using Spec = uint16_t;
51+
4952
private:
5053
static const unsigned NumSubclassDataBits = 24;
5154
static_assert(
@@ -63,7 +66,6 @@ class MCExpr {
6366
bool InSet) const;
6467

6568
protected:
66-
using Spec = uint16_t;
6769
explicit MCExpr(ExprKind Kind, SMLoc Loc, unsigned SubclassData = 0)
6870
: Kind(Kind), SubclassData(SubclassData), Loc(Loc) {
6971
assert(SubclassData < (1 << NumSubclassDataBits) &&

‎llvm/include/llvm/MC/MCInst.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "llvm/ADT/SmallVector.h"
1919
#include "llvm/ADT/StringRef.h"
2020
#include "llvm/ADT/bit.h"
21+
#include "llvm/MC/MCExpr.h"
2122
#include "llvm/MC/MCRegister.h"
2223
#include "llvm/Support/Compiler.h"
2324
#include "llvm/Support/SMLoc.h"
@@ -28,7 +29,6 @@
2829
namespace llvm {
2930

3031
class MCContext;
31-
class MCExpr;
3232
class MCInst;
3333
class MCInstPrinter;
3434
class MCRegisterInfo;
@@ -179,6 +179,7 @@ class MCOperand {
179179
LLVM_ABI void print(raw_ostream &OS, const MCContext *Ctx = nullptr) const;
180180
LLVM_ABI void dump() const;
181181
LLVM_ABI bool isBareSymbolRef() const;
182+
LLVM_ABI bool isSimpleSymbolRef(MCExpr::Spec &Specifier) const;
182183
LLVM_ABI bool evaluateAsConstantImm(int64_t &Imm) const;
183184
};
184185

‎llvm/lib/MC/MCInst.cpp‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,29 @@ bool MCOperand::evaluateAsConstantImm(int64_t &Imm) const {
6363
}
6464

6565
bool MCOperand::isBareSymbolRef() const {
66-
assert(isExpr() &&
67-
"isBareSymbolRef expects only expressions");
66+
MCExpr::Spec Specifier;
67+
return isSimpleSymbolRef(Specifier) && Specifier == 0;
68+
}
69+
70+
bool MCOperand::isSimpleSymbolRef(MCExpr::Spec &Specifier) const {
71+
assert(isExpr() && "isBareSymbolRef expects only expressions");
6872
const MCExpr *Expr = getExpr();
6973
MCExpr::ExprKind Kind = getExpr()->getKind();
70-
return Kind == MCExpr::SymbolRef &&
71-
cast<MCSymbolRefExpr>(Expr)->getSpecifier() == 0;
74+
75+
switch (Kind) {
76+
case MCExpr::Binary:
77+
case MCExpr::Unary:
78+
case MCExpr::Constant:
79+
case MCExpr::Target:
80+
break;
81+
case MCExpr::SymbolRef:
82+
Specifier = cast<MCSymbolRefExpr>(Expr)->getSpecifier();
83+
return true;
84+
case MCExpr::Specifier:
85+
Specifier = cast<MCSpecifierExpr>(Expr)->getSpecifier();
86+
return true;
87+
}
88+
return false;
7289
}
7390

7491
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,13 @@
1212
//===----------------------------------------------------------------------===//
1313

1414
#include "RISCVBaseInfo.h"
15+
#include "MCTargetDesc/RISCVMCAsmInfo.h"
16+
#include "llvm/BinaryFormat/ELF.h"
17+
#include "llvm/MC/MCExpr.h"
1518
#include "llvm/MC/MCInst.h"
1619
#include "llvm/MC/MCRegisterInfo.h"
1720
#include "llvm/MC/MCSubtargetInfo.h"
21+
#include "llvm/Support/Casting.h"
1822
#include "llvm/Support/raw_ostream.h"
1923
#include "llvm/TargetParser/TargetParser.h"
2024
#include "llvm/TargetParser/Triple.h"

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
//===----------------------------------------------------------------------===//
1212

1313
#include "RISCVInstPrinter.h"
14+
#include "MCTargetDesc/RISCVMCAsmInfo.h"
1415
#include "RISCVBaseInfo.h"
16+
#include "llvm/BinaryFormat/ELF.h"
1517
#include "llvm/MC/MCAsmInfo.h"
1618
#include "llvm/MC/MCExpr.h"
1719
#include "llvm/MC/MCInst.h"
1820
#include "llvm/MC/MCInstPrinter.h"
1921
#include "llvm/MC/MCSubtargetInfo.h"
2022
#include "llvm/MC/MCSymbol.h"
23+
#include "llvm/Support/Casting.h"
2124
#include "llvm/Support/CommandLine.h"
2225
#include "llvm/Support/ErrorHandling.h"
2326
using namespace llvm;

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

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -327,12 +327,20 @@ def uimm16 : RISCVUImmOp<16>;
327327
def uimm32 : RISCVUImmOp<32>;
328328
def uimm48 : RISCVUImmOp<48>;
329329
def uimm64 : RISCVUImmOp<64>;
330+
330331
def simm12 : RISCVSImmLeafOp<12> {
331332
let MCOperandPredicate = [{
332333
int64_t Imm;
333334
if (MCOp.evaluateAsConstantImm(Imm))
334335
return isInt<12>(Imm);
335-
return MCOp.isBareSymbolRef();
336+
337+
MCExpr::Spec S;
338+
if (!MCOp.isExpr() || !MCOp.isSimpleSymbolRef(S))
339+
return false;
340+
341+
return (S == RISCV::S_LO) || (S == RISCV::S_PCREL_LO) ||
342+
(S == RISCV::S_TPREL_LO) || (S == ELF::R_RISCV_TLSDESC_LOAD_LO12) ||
343+
(S == ELF::R_RISCV_TLSDESC_ADD_LO12);
336344
}];
337345
}
338346

@@ -368,20 +376,37 @@ def bare_simm13_lsb0 : BareSImm13Lsb0MaybeSym,
368376
// is used to match with a basic block (eg. BccPat<>).
369377
def bare_simm13_lsb0_bb : BareSImm13Lsb0MaybeSym;
370378

371-
class UImm20OperandMaybeSym : RISCVUImmOp<20> {
379+
def uimm20_lui : RISCVUImmOp<20> {
380+
let ParserMatchClass = UImmAsmOperand<20, "LUI">;
372381
let MCOperandPredicate = [{
373382
int64_t Imm;
374383
if (MCOp.evaluateAsConstantImm(Imm))
375384
return isUInt<20>(Imm);
376-
return MCOp.isBareSymbolRef();
377-
}];
378-
}
379385

380-
def uimm20_lui : UImm20OperandMaybeSym {
381-
let ParserMatchClass = UImmAsmOperand<20, "LUI">;
386+
MCExpr::Spec S;
387+
if (!MCOp.isExpr() || !MCOp.isSimpleSymbolRef(S))
388+
return false;
389+
390+
return (S == ELF::R_RISCV_HI20) ||
391+
(S == ELF::R_RISCV_TPREL_HI20);
392+
}];
382393
}
383-
def uimm20_auipc : UImm20OperandMaybeSym {
394+
def uimm20_auipc : RISCVUImmOp<20> {
384395
let ParserMatchClass = UImmAsmOperand<20, "AUIPC">;
396+
let MCOperandPredicate = [{
397+
int64_t Imm;
398+
if (MCOp.evaluateAsConstantImm(Imm))
399+
return isUInt<20>(Imm);
400+
401+
MCExpr::Spec S;
402+
if (!MCOp.isExpr() || !MCOp.isSimpleSymbolRef(S))
403+
return false;
404+
405+
return (S == ELF::R_RISCV_PCREL_HI20) || (S == ELF::R_RISCV_GOT_HI20) ||
406+
(S == ELF::R_RISCV_TLS_GOT_HI20) ||
407+
(S == ELF::R_RISCV_TLS_GD_HI20) ||
408+
(S == ELF::R_RISCV_TLSDESC_HI20);
409+
}];
385410
}
386411

387412
def uimm20 : RISCVUImmOp<20>;

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,9 @@ def uimmlog2xlennonzero : RISCVOp, ImmLeaf<XLenVT, [{
3737
def simm6 : RISCVSImmLeafOp<6> {
3838
let MCOperandPredicate = [{
3939
int64_t Imm;
40-
if (MCOp.evaluateAsConstantImm(Imm))
41-
return isInt<6>(Imm);
42-
return MCOp.isBareSymbolRef();
40+
if (!MCOp.evaluateAsConstantImm(Imm))
41+
return false;
42+
return isInt<6>(Imm);
4343
}];
4444
}
4545

@@ -51,9 +51,9 @@ def simm6nonzero : RISCVOp,
5151
let OperandType = "OPERAND_SIMM6_NONZERO";
5252
let MCOperandPredicate = [{
5353
int64_t Imm;
54-
if (MCOp.evaluateAsConstantImm(Imm))
55-
return (Imm != 0) && isInt<6>(Imm);
56-
return MCOp.isBareSymbolRef();
54+
if (!MCOp.evaluateAsConstantImm(Imm))
55+
return false;
56+
return (Imm != 0) && isInt<6>(Imm);
5757
}];
5858
}
5959

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ def VMaskCarryInOp : RegisterOperand<VMV0> {
7272
def simm5 : RISCVSImmLeafOp<5> {
7373
let MCOperandPredicate = [{
7474
int64_t Imm;
75-
if (MCOp.evaluateAsConstantImm(Imm))
76-
return isInt<5>(Imm);
77-
return MCOp.isBareSymbolRef();
75+
if (!MCOp.evaluateAsConstantImm(Imm))
76+
return false;
77+
return isInt<5>(Imm);
7878
}];
7979
}
8080

@@ -84,9 +84,9 @@ def simm5_plus1 : RISCVOp, ImmLeaf<XLenVT,
8484
let OperandType = "OPERAND_SIMM5_PLUS1";
8585
let MCOperandPredicate = [{
8686
int64_t Imm;
87-
if (MCOp.evaluateAsConstantImm(Imm))
88-
return (isInt<5>(Imm) && Imm != -16) || Imm == 16;
89-
return MCOp.isBareSymbolRef();
87+
if (!MCOp.evaluateAsConstantImm(Imm))
88+
return false;
89+
return (isInt<5>(Imm) && Imm != -16) || Imm == 16;
9090
}];
9191
}
9292

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ def tsimm5 : Operand<XLenVT>, TImmLeaf<XLenVT, [{return isInt<5>(Imm);}]> {
4343
let DecoderMethod = "decodeSImmOperand<5>";
4444
let MCOperandPredicate = [{
4545
int64_t Imm;
46-
if (MCOp.evaluateAsConstantImm(Imm))
47-
return isInt<5>(Imm);
48-
return MCOp.isBareSymbolRef();
46+
if (!MCOp.evaluateAsConstantImm(Imm))
47+
return false;
48+
return isInt<5>(Imm);
4949
}];
5050
}
5151

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,12 @@ def simm20_li : RISCVOp<XLenVT>,
146146
int64_t Imm;
147147
if (MCOp.evaluateAsConstantImm(Imm))
148148
return isInt<20>(Imm);
149-
return MCOp.isBareSymbolRef();
149+
150+
MCExpr::Spec S;
151+
if (!MCOp.isExpr() || !MCOp.isSimpleSymbolRef(S))
152+
return false;
153+
154+
return S == RISCV::S_QC_ABS20;
150155
}];
151156
}
152157

0 commit comments

Comments
 (0)