Skip to content

Commit

Permalink
[RISCV] Check that the stack adjust immediate for cm.push/pop* has th…
Browse files Browse the repository at this point in the history
…e correct sign and is divisible by 16. (llvm#85295)

To do this I've added a new AsmOperand for cm.push to expect a negative
value. We also use that to customize the print function so that we don't
need to detect cm.push opcode to add the negative sign.

I've renamed some places that used Spimm to be StackAdj since that's
what is being parsed. I'm still not about where we should use Spimm or
StackAdj.

I've removed the printSpimm helper function which in one usage printed
the sp[5:4]<<4 value and the other usage printed the full stack
adjustment. There wasn't anything interesting about how it was printed
it just passed the value to the raw_stream. If there was something
special needed, it's unclear whether it would be the same for the two
different usages so I inlined it.

One open question is whether we need to support stack adjustments
expressed as an expression rather than a literal integer.
  • Loading branch information
topperc authored Mar 26, 2024
1 parent ac1af75 commit 1584014
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 36 deletions.
18 changes: 12 additions & 6 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,11 @@ class RISCVAsmParser : public MCTargetAsmParser {
ParseStatus parseReglist(OperandVector &Operands);
ParseStatus parseRegReg(OperandVector &Operands);
ParseStatus parseRetval(OperandVector &Operands);
ParseStatus parseZcmpSpimm(OperandVector &Operands);
ParseStatus parseZcmpStackAdj(OperandVector &Operands,
bool ExpectNegative = false);
ParseStatus parseZcmpNegStackAdj(OperandVector &Operands) {
return parseZcmpStackAdj(Operands, /*ExpectNegative*/ true);
}

bool parseOperand(OperandVector &Operands, StringRef Mnemonic);

Expand Down Expand Up @@ -1062,7 +1066,7 @@ struct RISCVOperand final : public MCParsedAsmOperand {
break;
case KindTy::Spimm:
OS << "<Spimm: ";
RISCVZC::printSpimm(Spimm.Val, OS);
OS << Spimm.Val;
OS << '>';
break;
case KindTy::RegReg:
Expand Down Expand Up @@ -1608,7 +1612,7 @@ bool RISCVAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
ErrorLoc,
"operand must be {ra [, s0[-sN]]} or {x1 [, x8[-x9][, x18[-xN]]]}");
}
case Match_InvalidSpimm: {
case Match_InvalidStackAdj: {
SMLoc ErrorLoc = ((RISCVOperand &)*Operands[ErrorInfo]).getStartLoc();
return Error(
ErrorLoc,
Expand Down Expand Up @@ -2583,16 +2587,18 @@ ParseStatus RISCVAsmParser::parseReglist(OperandVector &Operands) {
return ParseStatus::Success;
}

ParseStatus RISCVAsmParser::parseZcmpSpimm(OperandVector &Operands) {
(void)parseOptionalToken(AsmToken::Minus);
ParseStatus RISCVAsmParser::parseZcmpStackAdj(OperandVector &Operands,
bool ExpectNegative) {
bool Negative = parseOptionalToken(AsmToken::Minus);

SMLoc S = getLoc();
int64_t StackAdjustment = getLexer().getTok().getIntVal();
unsigned Spimm = 0;
unsigned RlistVal = static_cast<RISCVOperand *>(Operands[1].get())->Rlist.Val;

bool IsEABI = isRVE();
if (!RISCVZC::getSpimm(RlistVal, Spimm, StackAdjustment, isRV64(), IsEABI))
if (Negative != ExpectNegative ||
!RISCVZC::getSpimm(RlistVal, Spimm, StackAdjustment, isRV64(), IsEABI))
return ParseStatus::NoMatch;
Operands.push_back(RISCVOperand::createSpimm(Spimm << 4, S));
getLexer().Lex();
Expand Down
2 changes: 0 additions & 2 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,4 @@ void RISCVZC::printRlist(unsigned SlistEncode, raw_ostream &OS) {
OS << "}";
}

void RISCVZC::printSpimm(int64_t Spimm, raw_ostream &OS) { OS << Spimm; }

} // namespace llvm
8 changes: 5 additions & 3 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -580,15 +580,17 @@ inline static bool getSpimm(unsigned RlistVal, unsigned &SpimmVal,
int64_t StackAdjustment, bool IsRV64, bool IsEABI) {
if (RlistVal == RLISTENCODE::INVALID_RLIST)
return false;
unsigned stackAdj = getStackAdjBase(RlistVal, IsRV64, IsEABI);
SpimmVal = (StackAdjustment - stackAdj) / 16;
unsigned StackAdjBase = getStackAdjBase(RlistVal, IsRV64, IsEABI);
StackAdjustment -= StackAdjBase;
if (StackAdjustment % 16 != 0)
return false;
SpimmVal = StackAdjustment / 16;
if (SpimmVal > 3)
return false;
return true;
}

void printRlist(unsigned SlistEncode, raw_ostream &OS);
void printSpimm(int64_t Spimm, raw_ostream &OS);
} // namespace RISCVZC

} // namespace llvm
Expand Down
19 changes: 10 additions & 9 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -292,24 +292,25 @@ void RISCVInstPrinter::printRegReg(const MCInst *MI, unsigned OpNo,
O << ")";
}

void RISCVInstPrinter::printSpimm(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI, raw_ostream &O) {
void RISCVInstPrinter::printStackAdj(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI, raw_ostream &O,
bool Negate) {
int64_t Imm = MI->getOperand(OpNo).getImm();
unsigned Opcode = MI->getOpcode();
bool IsRV64 = STI.hasFeature(RISCV::Feature64Bit);
bool IsEABI = STI.hasFeature(RISCV::FeatureRVE);
int64_t Spimm = 0;
int64_t StackAdj = 0;
auto RlistVal = MI->getOperand(0).getImm();
assert(RlistVal != 16 && "Incorrect rlist.");
auto Base = RISCVZC::getStackAdjBase(RlistVal, IsRV64, IsEABI);
Spimm = Imm + Base;
assert((Spimm >= Base && Spimm <= Base + 48) && "Incorrect spimm");
if (Opcode == RISCV::CM_PUSH)
Spimm = -Spimm;
StackAdj = Imm + Base;
assert((StackAdj >= Base && StackAdj <= Base + 48) &&
"Incorrect stack adjust");
if (Negate)
StackAdj = -StackAdj;

// RAII guard for ANSI color escape sequences
WithMarkup ScopedMarkup = markup(O, Markup::Immediate);
RISCVZC::printSpimm(Spimm, O);
O << StackAdj;
}

void RISCVInstPrinter::printVMaskReg(const MCInst *MI, unsigned OpNo,
Expand Down
9 changes: 7 additions & 2 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVInstPrinter.h
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,13 @@ class RISCVInstPrinter : public MCInstPrinter {
const MCSubtargetInfo &STI, raw_ostream &O);
void printRlist(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
raw_ostream &O);
void printSpimm(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
raw_ostream &O);
void printStackAdj(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI, raw_ostream &O,
bool Negate = false);
void printNegStackAdj(const MCInst *MI, unsigned OpNo,
const MCSubtargetInfo &STI, raw_ostream &O) {
return printStackAdj(MI, OpNo, STI, O, /*Negate*/ true);
}
void printRegReg(const MCInst *MI, unsigned OpNo, const MCSubtargetInfo &STI,
raw_ostream &O);
// Autogenerated by tblgen.
Expand Down
51 changes: 37 additions & 14 deletions llvm/lib/Target/RISCV/RISCVInstrInfoZc.td
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,20 @@ def RlistAsmOperand : AsmOperandClass {
let DiagnosticType = "InvalidRlist";
}

def SpimmAsmOperand : AsmOperandClass {
let Name = "Spimm";
let ParserMethod = "parseZcmpSpimm";
let DiagnosticType = "InvalidSpimm";
def StackAdjAsmOperand : AsmOperandClass {
let Name = "StackAdj";
let ParserMethod = "parseZcmpStackAdj";
let DiagnosticType = "InvalidStackAdj";
let PredicateMethod = "isSpimm";
let RenderMethod = "addSpimmOperands";
}

def NegStackAdjAsmOperand : AsmOperandClass {
let Name = "NegStackAdj";
let ParserMethod = "parseZcmpNegStackAdj";
let DiagnosticType = "InvalidStackAdj";
let PredicateMethod = "isSpimm";
let RenderMethod = "addSpimmOperands";
}

def rlist : Operand<OtherVT> {
Expand All @@ -59,11 +69,23 @@ def rlist : Operand<OtherVT> {
// 0~3 Reserved for EABI
return isUInt<4>(Imm) && Imm >= 4;
}];
}
}

def stackadj : Operand<OtherVT> {
let ParserMatchClass = StackAdjAsmOperand;
let PrintMethod = "printStackAdj";
let DecoderMethod = "decodeZcmpSpimm";
let MCOperandPredicate = [{
int64_t Imm;
if (!MCOp.evaluateAsConstantImm(Imm))
return false;
return isShiftedUInt<2, 4>(Imm);
}];
}

def spimm : Operand<OtherVT> {
let ParserMatchClass = SpimmAsmOperand;
let PrintMethod = "printSpimm";
def negstackadj : Operand<OtherVT> {
let ParserMatchClass = NegStackAdjAsmOperand;
let PrintMethod = "printNegStackAdj";
let DecoderMethod = "decodeZcmpSpimm";
let MCOperandPredicate = [{
int64_t Imm;
Expand Down Expand Up @@ -124,14 +146,15 @@ class RVZcArith_r<bits<5> funct5, string OpcodeStr> :
let Constraints = "$rd = $rd_wb";
}

class RVInstZcCPPP<bits<5> funct5, string opcodestr>
: RVInst16<(outs), (ins rlist:$rlist, spimm:$spimm),
opcodestr, "$rlist, $spimm", [], InstFormatOther> {
class RVInstZcCPPP<bits<5> funct5, string opcodestr,
DAGOperand immtype = stackadj>
: RVInst16<(outs), (ins rlist:$rlist, immtype:$stackadj),
opcodestr, "$rlist, $stackadj", [], InstFormatOther> {
bits<4> rlist;
bits<16> spimm;
bits<16> stackadj;

let Inst{1-0} = 0b10;
let Inst{3-2} = spimm{5-4};
let Inst{3-2} = stackadj{5-4};
let Inst{7-4} = rlist;
let Inst{12-8} = funct5;
let Inst{15-13} = 0b101;
Expand Down Expand Up @@ -195,7 +218,7 @@ def CM_MVSA01 : RVInst16CA<0b101011, 0b01, 0b10, (outs SR07:$rs1, SR07:$rs2),

let DecoderNamespace = "RVZcmp", Predicates = [HasStdExtZcmp] in {
let hasSideEffects = 0, mayLoad = 0, mayStore = 1, Uses = [X2], Defs = [X2] in
def CM_PUSH : RVInstZcCPPP<0b11000, "cm.push">,
def CM_PUSH : RVInstZcCPPP<0b11000, "cm.push", negstackadj>,
Sched<[WriteIALU, ReadIALU, ReadStoreData, ReadStoreData,
ReadStoreData, ReadStoreData, ReadStoreData, ReadStoreData,
ReadStoreData, ReadStoreData, ReadStoreData, ReadStoreData,
Expand Down
12 changes: 12 additions & 0 deletions llvm/test/MC/RISCV/rv32zcmp-invalid.s
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,15 @@ cm.popretz {ra, s0-s10}, 112

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.popretz {ra, s0-s1}, 112

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.push {ra}, 16

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.pop {ra, s0-s1}, -32

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.push {ra}, -8

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.pop {ra, s0-s1}, -40
12 changes: 12 additions & 0 deletions llvm/test/MC/RISCV/rv64zcmp-invalid.s
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,15 @@ cm.popretz {ra, s0-s10}, 112

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.popretz {ra, s0-s1}, 112

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.push {ra}, 16

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.pop {ra, s0-s1}, -32

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.push {ra}, -15

# CHECK-ERROR: error: stack adjustment is invalid for this instruction and register list; refer to Zc spec for a detailed range of stack adjustment
cm.pop {ra, s0-s1}, -33

0 comments on commit 1584014

Please sign in to comment.