Skip to content

Commit 9d34667

Browse files
MaskRayMaskRay
committed
RISCVAsmParser: Don't treat operands with relocation specifier as parse-time constants
An immediate operand is encoded as an `MCExpr`, with `RISCVMCExpr` specifying an operand that includes a relocation specifier. When https://reviews.llvm.org/D23568 added initial fixup and relocation support in 2017, it adapted code from `PPCMCExpr` (for `@l` `@ha`) to evaluate the `RISCVMCExpr` operand. (PPCAsmParser had considerable technical debt, though I’ve recently streamlined it somewhat, e.g. 8560da2) Evaluating RISCVMCExpr during parsing is unnecessary. For example, there's no need to treat `lui a0, %hi(2)` differently from `lui a0, %hi(foo)` when foo has not been defined yet. This evaluation introduces unnecessary complexity. For instance, parser functions require an extra check like `VK == RISCVMCExpr::VK_None`, as seen in these examples: ``` if (!evaluateConstantImm(getImm(), Imm, VK) || VK != RISCVMCExpr::VK_None) return IsConstantImm && isUInt<N>(Imm) && VK == RISCVMCExpr::VK_None; ``` This PR eliminates the parse-time evaluation of `RISCVMCExpr`, aligning it more closely with other targets. I will update the remaining MC/RISCV tests if this PR looks good. Pull Request: llvm#133377
1 parent 6b1acdb commit 9d34667

File tree

9 files changed

+69
-111
lines changed

9 files changed

+69
-111
lines changed

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

Lines changed: 25 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -524,15 +524,8 @@ struct RISCVOperand final : public MCParsedAsmOperand {
524524
bool isGPRAsFPR32() const { return isGPRF32() && Reg.IsGPRAsFPR; }
525525
bool isGPRPairAsFPR64() const { return isGPRPair() && Reg.IsGPRAsFPR; }
526526

527-
static bool evaluateConstantImm(const MCExpr *Expr, int64_t &Imm,
528-
RISCVMCExpr::Specifier &VK) {
529-
if (auto *RE = dyn_cast<RISCVMCExpr>(Expr)) {
530-
VK = RE->getSpecifier();
531-
return RE->evaluateAsConstant(Imm);
532-
}
533-
527+
static bool evaluateConstantImm(const MCExpr *Expr, int64_t &Imm) {
534528
if (auto CE = dyn_cast<MCConstantExpr>(Expr)) {
535-
VK = RISCVMCExpr::VK_None;
536529
Imm = CE->getValue();
537530
return true;
538531
}
@@ -547,7 +540,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
547540
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
548541
if (!isImm())
549542
return false;
550-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
543+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
551544
bool IsValid;
552545
if (!IsConstantImm)
553546
IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK);
@@ -562,7 +555,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
562555
int64_t Imm;
563556
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
564557
// Must be of 'immediate' type but not a constant.
565-
if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
558+
if (!isImm() || evaluateConstantImm(getImm(), Imm))
566559
return false;
567560
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
568561
VK == RISCVMCExpr::VK_None;
@@ -572,7 +565,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
572565
int64_t Imm;
573566
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
574567
// Must be of 'immediate' type but not a constant.
575-
if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
568+
if (!isImm() || evaluateConstantImm(getImm(), Imm))
576569
return false;
577570
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
578571
(VK == RISCVMCExpr::VK_CALL || VK == RISCVMCExpr::VK_CALL_PLT);
@@ -582,7 +575,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
582575
int64_t Imm;
583576
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
584577
// Must be of 'immediate' type but not a constant.
585-
if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
578+
if (!isImm() || evaluateConstantImm(getImm(), Imm))
586579
return false;
587580
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
588581
VK == RISCVMCExpr::VK_CALL;
@@ -592,7 +585,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
592585
int64_t Imm;
593586
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
594587
// Must be of 'immediate' type but not a constant.
595-
if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
588+
if (!isImm() || evaluateConstantImm(getImm(), Imm))
596589
return false;
597590
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
598591
VK == RISCVMCExpr::VK_TPREL_ADD;
@@ -602,7 +595,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
602595
int64_t Imm;
603596
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
604597
// Must be of 'immediate' type but not a constant.
605-
if (!isImm() || evaluateConstantImm(getImm(), Imm, VK))
598+
if (!isImm() || evaluateConstantImm(getImm(), Imm))
606599
return false;
607600
return RISCVAsmParser::classifySymbolRef(getImm(), VK) &&
608601
VK == RISCVMCExpr::VK_TLSDESC_CALL;
@@ -650,7 +643,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
650643
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
651644
if (!isImm())
652645
return false;
653-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
646+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
654647
if (VK == RISCVMCExpr::VK_LO || VK == RISCVMCExpr::VK_PCREL_LO ||
655648
VK == RISCVMCExpr::VK_TLSDESC_LOAD_LO ||
656649
VK == RISCVMCExpr::VK_TLSDESC_ADD_LO)
@@ -667,41 +660,36 @@ struct RISCVOperand final : public MCParsedAsmOperand {
667660

668661
bool isImmXLenLI_Restricted() const {
669662
int64_t Imm;
670-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
671663
if (!isImm())
672664
return false;
673-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
665+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
674666
// 'la imm' supports constant immediates only.
675-
return IsConstantImm && (VK == RISCVMCExpr::VK_None) &&
667+
return IsConstantImm &&
676668
(isRV64Imm() || (isInt<32>(Imm) || isUInt<32>(Imm)));
677669
}
678670

679671
template <unsigned N> bool isUImm() const {
680672
int64_t Imm;
681-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
682673
if (!isImm())
683674
return false;
684-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
685-
return IsConstantImm && isUInt<N>(Imm) && VK == RISCVMCExpr::VK_None;
675+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
676+
return IsConstantImm && isUInt<N>(Imm);
686677
}
687678

688679
template <unsigned N, unsigned S> bool isUImmShifted() const {
689680
int64_t Imm;
690-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
691681
if (!isImm())
692682
return false;
693-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
694-
return IsConstantImm && isShiftedUInt<N, S>(Imm) &&
695-
VK == RISCVMCExpr::VK_None;
683+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
684+
return IsConstantImm && isShiftedUInt<N, S>(Imm);
696685
}
697686

698687
template <class Pred> bool isUImmPred(Pred p) const {
699688
int64_t Imm;
700-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
701689
if (!isImm())
702690
return false;
703-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
704-
return IsConstantImm && p(Imm) && VK == RISCVMCExpr::VK_None;
691+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
692+
return IsConstantImm && p(Imm);
705693
}
706694

707695
bool isUImmLog2XLen() const {
@@ -789,22 +777,18 @@ struct RISCVOperand final : public MCParsedAsmOperand {
789777

790778
template <unsigned N> bool isSImm() const {
791779
int64_t Imm;
792-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
793780
if (!isImm())
794781
return false;
795-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
796-
return IsConstantImm && isInt<N>(fixImmediateForRV32(Imm, isRV64Imm())) &&
797-
VK == RISCVMCExpr::VK_None;
782+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
783+
return IsConstantImm && isInt<N>(fixImmediateForRV32(Imm, isRV64Imm()));
798784
}
799785

800786
template <class Pred> bool isSImmPred(Pred p) const {
801787
int64_t Imm;
802-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
803788
if (!isImm())
804789
return false;
805-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
806-
return IsConstantImm && p(fixImmediateForRV32(Imm, isRV64Imm())) &&
807-
VK == RISCVMCExpr::VK_None;
790+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
791+
return IsConstantImm && p(fixImmediateForRV32(Imm, isRV64Imm()));
808792
}
809793

810794
bool isSImm5() const { return isSImm<5>(); }
@@ -865,7 +849,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
865849
bool IsValid;
866850
if (!isImm())
867851
return false;
868-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
852+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
869853
if (!IsConstantImm)
870854
IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK);
871855
else
@@ -905,7 +889,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
905889
bool IsValid;
906890
if (!isImm())
907891
return false;
908-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
892+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
909893
if (!IsConstantImm) {
910894
IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK);
911895
return IsValid &&
@@ -923,7 +907,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
923907
bool IsValid;
924908
if (!isImm())
925909
return false;
926-
bool IsConstantImm = evaluateConstantImm(getImm(), Imm, VK);
910+
bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
927911
if (!IsConstantImm) {
928912
IsValid = RISCVAsmParser::classifySymbolRef(getImm(), VK);
929913
return IsValid &&
@@ -1163,8 +1147,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
11631147
static void addExpr(MCInst &Inst, const MCExpr *Expr, bool IsRV64Imm) {
11641148
assert(Expr && "Expr shouldn't be null!");
11651149
int64_t Imm = 0;
1166-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
1167-
bool IsConstant = evaluateConstantImm(Expr, Imm, VK);
1150+
bool IsConstant = evaluateConstantImm(Expr, Imm);
11681151

11691152
if (IsConstant)
11701153
Inst.addOperand(
@@ -1213,9 +1196,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
12131196
assert(N == 1 && "Invalid number of operands!");
12141197
int64_t Imm = 0;
12151198
if (Kind == KindTy::Immediate) {
1216-
RISCVMCExpr::Specifier VK = RISCVMCExpr::VK_None;
1217-
[[maybe_unused]] bool IsConstantImm =
1218-
evaluateConstantImm(getImm(), Imm, VK);
1199+
[[maybe_unused]] bool IsConstantImm = evaluateConstantImm(getImm(), Imm);
12191200
assert(IsConstantImm && "Invalid VTypeI Operand!");
12201201
} else {
12211202
Imm = getVType();

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

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -164,30 +164,3 @@ StringRef RISCVMCExpr::getSpecifierName(Specifier S) {
164164
}
165165
llvm_unreachable("Invalid ELF symbol kind");
166166
}
167-
168-
bool RISCVMCExpr::evaluateAsConstant(int64_t &Res) const {
169-
MCValue Value;
170-
if (specifier != VK_LO && specifier != VK_HI)
171-
return false;
172-
173-
if (!getSubExpr()->evaluateAsRelocatable(Value, nullptr))
174-
return false;
175-
176-
if (!Value.isAbsolute())
177-
return false;
178-
179-
Res = evaluateAsInt64(Value.getConstant());
180-
return true;
181-
}
182-
183-
int64_t RISCVMCExpr::evaluateAsInt64(int64_t Value) const {
184-
switch (specifier) {
185-
default:
186-
llvm_unreachable("Invalid kind");
187-
case VK_LO:
188-
return SignExtend64<12>(Value);
189-
case VK_HI:
190-
// Add 1 if bit 11 is 1, to compensate for low 12 bits being negative.
191-
return ((Value + 0x800) >> 12) & 0xfffff;
192-
}
193-
}

‎llvm/test/MC/RISCV/insn.s‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -146,12 +146,12 @@ target:
146146
# CHECK-OBJ: fmadd.s fa0, fa1, fa2, fa3, rne
147147
.insn r4 MADD, 0, 0, fa0, fa1, fa2, fa3
148148

149-
# CHECK-ASM: .insn i 3, 5, t1, -2048(t2)
150-
# CHECK-ASM: encoding: [0x03,0xd3,0x03,0x80]
149+
# CHECK-ASM: .insn i 3, 5, t1, %lo(2048)(t2)
150+
# CHECK-ASM: encoding: [0x03,0xd3,0bAAAA0011,A]
151151
# CHECK-OBJ: lhu t1, -0x800(t2)
152152
.insn i 0x3, 0x5, x6, %lo(2048)(x7)
153-
# CHECK-ASM: .insn i 3, 5, t1, -2048(t2)
154-
# CHECK-ASM: encoding: [0x03,0xd3,0x03,0x80]
153+
# CHECK-ASM: .insn i 3, 5, t1, %lo(2048)(t2)
154+
# CHECK-ASM: encoding: [0x03,0xd3,0bAAAA0011,A]
155155
# CHECK-OBJ: lhu t1, -0x800(t2)
156156
.insn i LOAD, 0x5, x6, %lo(2048)(x7)
157157

‎llvm/test/MC/RISCV/rv32i-aliases-valid.s‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,9 @@ li x12, 0x80000000
8383
# CHECK-ALIAS: li a2, -1
8484
li x12, 0xFFFFFFFF
8585

86-
# CHECK-INST: addi a0, zero, 1110
87-
# CHECK-ALIAS: li a0, 1110
86+
# CHECK-ASM-NOALIAS: addi a0, zero, %lo(1193046)
87+
# CHECK-OBJ-NOALIAS: addi a0, zero, 1110
88+
# CHECK-ASM: addi a0, zero, %lo(1193046)
8889
li a0, %lo(0x123456)
8990

9091
# CHECK-OBJ-NOALIAS: addi a0, zero, 0

‎llvm/test/MC/RISCV/rv64i-valid.s‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ lwu x2, +4(x3)
1515
# CHECK-ASM-AND-OBJ: lwu tp, -2048(t0)
1616
# CHECK-ASM: encoding: [0x03,0xe2,0x02,0x80]
1717
lwu x4, -2048(x5)
18-
# CHECK-ASM-AND-OBJ: lwu t1, -2048(t2)
19-
# CHECK-ASM: encoding: [0x03,0xe3,0x03,0x80]
18+
# CHECK-ASM: lwu t1, %lo(2048)(t2) # encoding: [0x03,0xe3,0bAAAA0011,A]
19+
# CHECK-OBJ: lwu t1, -2048(t2)
2020
lwu x6, %lo(2048)(x7)
2121
# CHECK-ASM-AND-OBJ: lwu s0, 2047(s1)
2222
# CHECK-ASM: encoding: [0x03,0xe4,0xf4,0x7f]
@@ -25,8 +25,8 @@ lwu x8, 2047(x9)
2525
# CHECK-ASM-AND-OBJ: ld a0, -2048(a1)
2626
# CHECK-ASM: encoding: [0x03,0xb5,0x05,0x80]
2727
ld x10, -2048(x11)
28-
# CHECK-ASM-AND-OBJ: ld a2, -2048(a3)
29-
# CHECK-ASM: encoding: [0x03,0xb6,0x06,0x80]
28+
# CHECK-ASM: ld a2, %lo(2048)(a3) # encoding: [0x03,0xb6,0bAAAA0110,A]
29+
# CHECK-OBJ: ld a2, -2048(a3)
3030
ld x12, %lo(2048)(x13)
3131
# CHECK-ASM-AND-OBJ: ld a4, 2047(a5)
3232
# CHECK-ASM: encoding: [0x03,0xb7,0xf7,0x7f]
@@ -35,8 +35,8 @@ ld x14, 2047(x15)
3535
# CHECK-ASM-AND-OBJ: sd a6, -2048(a7)
3636
# CHECK-ASM: encoding: [0x23,0xb0,0x08,0x81]
3737
sd x16, -2048(x17)
38-
# CHECK-ASM-AND-OBJ: sd s2, -2048(s3)
39-
# CHECK-ASM: encoding: [0x23,0xb0,0x29,0x81]
38+
# CHECK-ASM: sd s2, %lo(2048)(s3) # encoding: [0x23'A',0xb0'A',0x29'A',0x01'A']
39+
# CHECK-OBJ: sd s2, -2048(s3)
4040
sd x18, %lo(2048)(x19)
4141
# CHECK-ASM-AND-OBJ: sd s4, 2047(s5)
4242
# CHECK-ASM: encoding: [0xa3,0xbf,0x4a,0x7f]

‎llvm/test/MC/RISCV/rvd-valid.s‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ fld f1, +4(ra)
2323
# CHECK-ASM-AND-OBJ: fld ft2, -2048(a3)
2424
# CHECK-ASM: encoding: [0x07,0xb1,0x06,0x80]
2525
fld f2, -2048(x13)
26-
# CHECK-ASM-AND-OBJ: fld ft3, -2048(s1)
27-
# CHECK-ASM: encoding: [0x87,0xb1,0x04,0x80]
26+
# CHECK-ASM: fld ft3, %lo(2048)(s1) # encoding: [0x87,0xb1,0bAAAA0100,A]
27+
# CHECK-OBJ: fld ft3, -2048(s1)
2828
fld f3, %lo(2048)(s1)
2929
# CHECK-ASM-AND-OBJ: fld ft4, 2047(s2)
3030
# CHECK-ASM: encoding: [0x07,0x32,0xf9,0x7f]
@@ -39,8 +39,8 @@ fsd f6, 2047(s4)
3939
# CHECK-ASM-AND-OBJ: fsd ft7, -2048(s5)
4040
# CHECK-ASM: encoding: [0x27,0xb0,0x7a,0x80]
4141
fsd f7, -2048(s5)
42-
# CHECK-ASM-AND-OBJ: fsd fs0, -2048(s6)
43-
# CHECK-ASM: encoding: [0x27,0x30,0x8b,0x80]
42+
# CHECK-ASM: fsd fs0, %lo(2048)(s6) # encoding: [0x27'A',0x30'A',0x8b'A',A]
43+
# CHECK-OBJ: fsd fs0, -2048(s6)
4444
fsd f8, %lo(2048)(s6)
4545
# CHECK-ASM-AND-OBJ: fsd fs1, 999(s7)
4646
# CHECK-ASM: encoding: [0xa7,0xb3,0x9b,0x3e]

‎llvm/test/MC/RISCV/rvf-valid.s‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ flw f1, +4(ra)
1818
# CHECK-ASM-AND-OBJ: flw ft2, -2048(a3)
1919
# CHECK-ASM: encoding: [0x07,0xa1,0x06,0x80]
2020
flw f2, -2048(x13)
21-
# CHECK-ASM-AND-OBJ: flw ft3, -2048(s1)
22-
# CHECK-ASM: encoding: [0x87,0xa1,0x04,0x80]
21+
# CHECK-ASM: flw ft3, %lo(2048)(s1) # encoding: [0x87,0xa1,0bAAAA0100,A]
22+
# CHECK-OBJ: flw ft3, -2048(s1)
2323
flw f3, %lo(2048)(s1)
2424
# CHECK-ASM-AND-OBJ: flw ft4, 2047(s2)
2525
# CHECK-ASM: encoding: [0x07,0x22,0xf9,0x7f]
@@ -34,8 +34,8 @@ fsw f6, 2047(s4)
3434
# CHECK-ASM-AND-OBJ: fsw ft7, -2048(s5)
3535
# CHECK-ASM: encoding: [0x27,0xa0,0x7a,0x80]
3636
fsw f7, -2048(s5)
37-
# CHECK-ASM-AND-OBJ: fsw fs0, -2048(s6)
38-
# CHECK-ASM: encoding: [0x27,0x20,0x8b,0x80]
37+
# CHECK-ASM: fsw fs0, %lo(2048)(s6) # encoding: [0x27'A',0x20'A',0x8b'A',A]
38+
# CHECK-OBJ: fsw fs0, -2048(s6)
3939
fsw f8, %lo(2048)(s6)
4040
# CHECK-ASM-AND-OBJ: fsw fs1, 999(s7)
4141
# CHECK-ASM: encoding: [0xa7,0xa3,0x9b,0x3e]

0 commit comments

Comments
 (0)