RPCS3/llvm-mirror
1
0
Fork 0
mirror of https://github.com/RPCS3/llvm-mirror.git synced 2024-12-14 07:09:08 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

[mips][ias] Check '$rs = $rd' constraints when both registers are in AsmText.

Browse Source 
Summary:
This is one possible solution to the problem of ignoring constraints that Simon
raised in D21473 but it's a bit of a hack.

The integrated assembler currently ignores violations of the tied register
constraints when the operands involved in a tie are both present in the AsmText.
For example, 'dati $rs, $rt, $imm' with the '$rs = $rt' will silently replace
$rt with $rs. So 'dati $2, $3, 1' is processed as if the user provided
'dati $2, $2, 1' without any diagnostic being emitted.

This is difficult to solve properly because there are multiple parts of the
matcher that are silently forcing these constraints to be met. Tied operands are
rendered to instructions by cloning previously rendered operands but this is
unnecessary because the matcher was already instructed to render the operand it
would have cloned. This is also unnecessary because earlier code has already
replaced the MCParsedOperand with the one it was tied to (so the parsed input
is matched as if it were 'dati <RegIdx 2>, <RegIdx 2>, <Imm 1>'). As a result,
it looks like fixing this properly amounts to a rewrite of the tied operand
handling which affects all targets.

This patch however, merely inserts a checking hook just before the
substitution of MCParsedOperands and the Mips target overrides it. It's not
possible to accurately check the registers are the same this early (because
numeric registers haven't been bound to a register class yet) so it cheats a
bit and checks that the tokens that produced the operand are lexically
identical. This works because tied registers need to have the same register
class but it does have a flaw. It will reject 'dati $4, $a0, 1' for violating
the constraint even though $a0 ends up as the same register as $4.

Reviewers: sdardis

Subscribers: dsanders, llvm-commits, sdardis

Differential Revision: https://reviews.llvm.org/D21994

llvm-svn: 276867
This commit is contained in:
Daniel Sanders 2016-07-27 13:49:44 +00:00
parent 5b3b0e70fd
commit 1eaf2533cd
8 changed files with 120 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
include/llvm/MC/MCParser/MCTargetAsmParser.h
View File

@ -196,6 +196,13 @@ public:
return Match_InvalidOperand;
}
/// Validate the instruction match against any complex target predicates
/// before rendering any operands to it.
virtual unsigned
checkEarlyTargetMatchPredicate(MCInst &Inst, const OperandVector &Operands) {
return Match_Success;
}
/// checkTargetMatchPredicate - Validate the instruction match against
/// any complex target predicates not expressible via match classes.
virtual unsigned checkTargetMatchPredicate(MCInst &Inst) {

122
lib/Target/Mips/AsmParser/MipsAsmParser.cpp
View File

@ -129,6 +129,9 @@ class MipsAsmParser : public MCTargetAsmParser {
#define GET_ASSEMBLER_HEADER
#include "MipsGenAsmMatcher.inc"
unsigned
checkEarlyTargetMatchPredicate(MCInst &Inst,
const OperandVector &Operands) override;
unsigned checkTargetMatchPredicate(MCInst &Inst) override;
bool MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
@ -395,6 +398,7 @@ public:
Match_RequiresDifferentSrcAndDst = FIRST_TARGET_MATCH_RESULT_TY,
Match_RequiresDifferentOperands,
Match_RequiresNoZeroRegister,
Match_RequiresSameSrcAndDst,
#define GET_OPERAND_DIAGNOSTIC_TYPES
#include "MipsGenAsmMatcher.inc"
#undef GET_OPERAND_DIAGNOSTIC_TYPES
@ -605,6 +609,7 @@ private:
struct RegIdxOp {
unsigned Index; /// Index into the register class
RegKind Kind; /// Bitfield of the kinds it could possibly be
struct Token Tok; /// The input token this operand originated from.
const MCRegisterInfo *RegInfo;
};
@ -632,7 +637,8 @@ private:
SMLoc StartLoc, EndLoc;
/// Internal constructor for register kinds
static std::unique_ptr<MipsOperand> CreateReg(unsigned Index, RegKind RegKind,
static std::unique_ptr<MipsOperand> CreateReg(unsigned Index, StringRef Str,
RegKind RegKind,
const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E,
MipsAsmParser &Parser) {
@ -640,6 +646,8 @@ private:
Op->RegIdx.Index = Index;
Op->RegIdx.RegInfo = RegInfo;
Op->RegIdx.Kind = RegKind;
Op->RegIdx.Tok.Data = Str.data();
Op->RegIdx.Tok.Length = Str.size();
Op->StartLoc = S;
Op->EndLoc = E;
return Op;
@ -1228,66 +1236,66 @@ public:
/// Create a numeric register (e.g. $1). The exact register remains
/// unresolved until an instruction successfully matches
static std::unique_ptr<MipsOperand>
createNumericReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S,
SMLoc E, MipsAsmParser &Parser) {
createNumericReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
DEBUG(dbgs() << "createNumericReg(" << Index << ", ...)\n");
return CreateReg(Index, RegKind_Numeric, RegInfo, S, E, Parser);
return CreateReg(Index, Str, RegKind_Numeric, RegInfo, S, E, Parser);
}
/// Create a register that is definitely a GPR.
/// This is typically only used for named registers such as $gp.
static std::unique_ptr<MipsOperand>
createGPRReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S, SMLoc E,
MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_GPR, RegInfo, S, E, Parser);
createGPRReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_GPR, RegInfo, S, E, Parser);
}
/// Create a register that is definitely a FGR.
/// This is typically only used for named registers such as $f0.
static std::unique_ptr<MipsOperand>
createFGRReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S, SMLoc E,
MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_FGR, RegInfo, S, E, Parser);
createFGRReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_FGR, RegInfo, S, E, Parser);
}
/// Create a register that is definitely a HWReg.
/// This is typically only used for named registers such as $hwr_cpunum.
static std::unique_ptr<MipsOperand>
createHWRegsReg(unsigned Index, const MCRegisterInfo *RegInfo,
createHWRegsReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_HWRegs, RegInfo, S, E, Parser);
return CreateReg(Index, Str, RegKind_HWRegs, RegInfo, S, E, Parser);
}
/// Create a register that is definitely an FCC.
/// This is typically only used for named registers such as $fcc0.
static std::unique_ptr<MipsOperand>
createFCCReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S, SMLoc E,
MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_FCC, RegInfo, S, E, Parser);
createFCCReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_FCC, RegInfo, S, E, Parser);
}
/// Create a register that is definitely an ACC.
/// This is typically only used for named registers such as $ac0.
static std::unique_ptr<MipsOperand>
createACCReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S, SMLoc E,
MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_ACC, RegInfo, S, E, Parser);
createACCReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_ACC, RegInfo, S, E, Parser);
}
/// Create a register that is definitely an MSA128.
/// This is typically only used for named registers such as $w0.
static std::unique_ptr<MipsOperand>
createMSA128Reg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S,
SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_MSA128, RegInfo, S, E, Parser);
createMSA128Reg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_MSA128, RegInfo, S, E, Parser);
}
/// Create a register that is definitely an MSACtrl.
/// This is typically only used for named registers such as $msaaccess.
static std::unique_ptr<MipsOperand>
createMSACtrlReg(unsigned Index, const MCRegisterInfo *RegInfo, SMLoc S,
SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, RegKind_MSACtrl, RegInfo, S, E, Parser);
createMSACtrlReg(unsigned Index, StringRef Str, const MCRegisterInfo *RegInfo,
SMLoc S, SMLoc E, MipsAsmParser &Parser) {
return CreateReg(Index, Str, RegKind_MSACtrl, RegInfo, S, E, Parser);
}
static std::unique_ptr<MipsOperand>
@ -1428,10 +1436,11 @@ public:
OS << ">";
break;
case k_RegisterIndex:
OS << "RegIdx<" << RegIdx.Index << ":" << RegIdx.Kind << ">";
OS << "RegIdx<" << RegIdx.Index << ":" << RegIdx.Kind << ", "
<< StringRef(RegIdx.Tok.Data, RegIdx.Tok.Length) << ">";
break;
case k_Token:
OS << Tok.Data;
OS << getToken();
break;
case k_RegList:
OS << "RegList< ";
@ -1444,6 +1453,22 @@ public:
break;
}
}
bool isValidForTie(const MipsOperand &Other) const {
if (Kind != Other.Kind)
return false;
switch (Kind) {
default:
llvm_unreachable("Unexpected kind");
return false;
case k_RegisterIndex: {
StringRef Token(RegIdx.Tok.Data, RegIdx.Tok.Length);
StringRef OtherToken(Other.RegIdx.Tok.Data, Other.RegIdx.Tok.Length);
return Token == OtherToken;
}
}
}
}; // class MipsOperand
} // namespace
@ -3685,6 +3710,20 @@ bool MipsAsmParser::expandAbs(MCInst &Inst, SMLoc IDLoc, MCStreamer &Out,
return false;
}
unsigned
MipsAsmParser::checkEarlyTargetMatchPredicate(MCInst &Inst,
const OperandVector &Operands) {
switch (Inst.getOpcode()) {
default:
return Match_Success;
case Mips::DATI:
case Mips::DAHI:
return static_cast<MipsOperand &>(*Operands[1])
.isValidForTie(static_cast<MipsOperand &>(*Operands[2]))
? Match_Success
: Match_RequiresSameSrcAndDst;
}
}
unsigned MipsAsmParser::checkTargetMatchPredicate(MCInst &Inst) {
switch (Inst.getOpcode()) {
// As described by the Mips32r2 spec, the registers Rd and Rs for
@ -3807,6 +3846,8 @@ bool MipsAsmParser::MatchAndEmitInstruction(SMLoc IDLoc, unsigned &Opcode,
return Error(IDLoc, "registers must be different");
case Match_RequiresNoZeroRegister:
return Error(IDLoc, "invalid operand ($zero) for instruction");
case Match_RequiresSameSrcAndDst:
return Error(IDLoc, "source and destination must match");
case Match_Immz:
return Error(RefineErrorLoc(IDLoc, Operands, ErrorInfo), "expected '0'");
case Match_UImm1_0:
@ -4398,8 +4439,8 @@ MipsAsmParser::parseMemOperand(OperandVector &Operands) {
// Zero register assumed, add a memory operand with ZERO as its base.
// "Base" will be managed by k_Memory.
auto Base = MipsOperand::createGPRReg(0, getContext().getRegisterInfo(),
S, E, *this);
auto Base = MipsOperand::createGPRReg(
0, "0", getContext().getRegisterInfo(), S, E, *this);
Operands.push_back(
MipsOperand::CreateMem(std::move(Base), IdVal, S, E, *this));
return MatchOperand_Success;
@ -4482,49 +4523,56 @@ MipsAsmParser::matchAnyRegisterNameWithoutDollar(OperandVector &Operands,
int Index = matchCPURegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createGPRReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchHWRegsRegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createHWRegsReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchFPURegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createFGRReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchFCCRegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createFCCReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchACRegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createACCReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchMSA128RegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createMSA128Reg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
Index = matchMSA128CtrlRegisterName(Identifier);
if (Index != -1) {
Operands.push_back(MipsOperand::createMSACtrlReg(
Index, getContext().getRegisterInfo(), S, getLexer().getLoc(), *this));
Index, Identifier, getContext().getRegisterInfo(), S,
getLexer().getLoc(), *this));
return MatchOperand_Success;
}
@ -4545,8 +4593,8 @@ MipsAsmParser::matchAnyRegisterWithoutDollar(OperandVector &Operands, SMLoc S) {
} else if (Token.is(AsmToken::Integer)) {
DEBUG(dbgs() << ".. integer\n");
Operands.push_back(MipsOperand::createNumericReg(
Token.getIntVal(), getContext().getRegisterInfo(), S, Token.getLoc(),
*this));
Token.getIntVal(), Token.getString(), getContext().getRegisterInfo(), S,
Token.getLoc(), *this));
return MatchOperand_Success;
}

2
lib/Target/Mips/Mips64r6InstrInfo.td
View File

@ -49,7 +49,7 @@ class SCD_R6_ENC : SPECIAL3_LL_SC_FM<OPCODE6_SCD>;
class AHI_ATI_DESC_BASE<string instr_asm, RegisterOperand GPROpnd, InstrItinClass itin> {
dag OutOperandList = (outs GPROpnd:$rs);
dag InOperandList = (ins GPROpnd:$rt, simm16_relaxed:$imm);
string AsmString = !strconcat(instr_asm, "\t$rt, $imm");
string AsmString = !strconcat(instr_asm, "\t$rt, $rs, $imm");
string Constraints = "$rs = $rt";
InstrItinClass Itinerary = itin;
}

2
test/MC/Disassembler/Mips/mips64r6/valid-mips64r6-el.txt
View File

@ -94,7 +94,7 @@
0x85 0x18 0x84 0x46 # CHECK: cmp.ult.s $f2, $f3, $f4
0x81 0x18 0xa4 0x46 # CHECK: cmp.un.d $f2, $f3, $f4
0x81 0x18 0x84 0x46 # CHECK: cmp.un.s $f2, $f3, $f4
0x78 0x56 0x66 0x04 # CHECK: dahi $3, 22136
0x78 0x56 0x66 0x04 # CHECK: dahi $3, $3, 22136
0x64 0x23 0x43 0x7c # CHECK: dalign $4, $2, $3, 5
0x34 0x12 0x62 0x74 # CHECK: daui $3, $2, 4660
0x24 0x20 0x02 0x7c # CHECK: dbitswap $4, $2

4
test/MC/Disassembler/Mips/mips64r6/valid-mips64r6.txt
View File

@ -50,8 +50,8 @@
0x03 0xe0 0x78 0x25 # CHECK: move $15, $ra
0x03 0xe0 0x78 0x2d # CHECK: move $15, $ra
0x04 0x11 0x14 0x9b # CHECK: bal 21104
0x04 0x66 0x56 0x78 # CHECK: dahi $3, 22136
0x04 0x7e 0xab 0xcd # CHECK: dati $3, -21555
0x04 0x66 0x56 0x78 # CHECK: dahi $3, $3, 22136
0x04 0x7e 0xab 0xcd # CHECK: dati $3, $3, -21555
# The encode/decode functions are not inverses of each other in the immediate case.
0x18 0x02 0x01 0x4d # CHECK: blezalc $2, 1336
0x18 0x02 0xff 0xfa # CHECk: blezalc $2, -20

1
test/MC/Mips/mips64r6/invalid.s
View File

@ -20,6 +20,7 @@ local_label:
break 7, -1 # CHECK: :[[@LINE]]:18: error: expected 10-bit unsigned immediate
break 7, 1024 # CHECK: :[[@LINE]]:18: error: expected 10-bit unsigned immediate
break 1024, 1024 # CHECK: :[[@LINE]]:15: error: expected 10-bit unsigned immediate
dati $2, $3, 1 # CHECK: :[[@LINE]]:9: error: source and destination must match
lh $33, 8($4) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhe $34, 8($2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction
lhu $35, 8($2) # CHECK: :[[@LINE]]:{{[0-9]+}}: error: invalid operand for instruction

12
test/MC/Mips/mips64r6/valid.s
View File

@ -101,12 +101,12 @@ a:
cmp.ult.s $f2,$f3,$f4 # CHECK: cmp.ult.s $f2, $f3, $f4 # encoding: [0x46,0x84,0x18,0x85]
cmp.un.d $f2,$f3,$f4 # CHECK: cmp.un.d $f2, $f3, $f4 # encoding: [0x46,0xa4,0x18,0x81]
cmp.un.s $f2,$f3,$f4 # CHECK: cmp.un.s $f2, $f3, $f4 # encoding: [0x46,0x84,0x18,0x81]
daddu $19,26943 # CHECK: daddiu $19, $19, 26943 # encoding: [0x66,0x73,0x69,0x3f]
daddu $24,$2,18079 # CHECK: daddiu $24, $2, 18079 # encoding: [0x64,0x58,0x46,0x9f]
dahi $3,0x5678 # CHECK: dahi $3, 22136 # encoding: [0x04,0x66,0x56,0x78]
dalign $4,$2,$3,5 # CHECK: dalign $4, $2, $3, 5 # encoding: [0x7c,0x43,0x23,0x64]
dati $3,0xabcd # CHECK: dati $3, -21555 # encoding: [0x04,0x7e,0xab,0xcd]
daui $3,$2,0x1234 # CHECK: daui $3, $2, 4660 # encoding: [0x74,0x62,0x12,0x34]
daddu $19,26943 # CHECK: daddiu $19, $19, 26943 # encoding: [0x66,0x73,0x69,0x3f]
daddu $24,$2,18079 # CHECK: daddiu $24, $2, 18079 # encoding: [0x64,0x58,0x46,0x9f]
dahi $3,$3,0x5678 # CHECK: dahi $3, $3, 22136 # encoding: [0x04,0x66,0x56,0x78]
dalign $4,$2,$3,5 # CHECK: dalign $4, $2, $3, 5 # encoding: [0x7c,0x43,0x23,0x64]
dati $3,$3,0xabcd # CHECK: dati $3, $3, -21555 # encoding: [0x04,0x7e,0xab,0xcd]
daui $3,$2,0x1234 # CHECK: daui $3, $2, 4660 # encoding: [0x74,0x62,0x12,0x34]
dbitswap $4, $2 # CHECK: dbitswap $4, $2 # encoding: [0x7c,0x02,0x20,0x24]
dclo $s2,$a2 # CHECK: dclo $18, $6 # encoding: [0x00,0xc0,0x90,0x53]
dclz $s0,$25 # CHECK: dclz $16, $25 # encoding: [0x03,0x20,0x80,0x52]

19
utils/TableGen/AsmMatcherEmitter.cpp
View File

@ -3254,8 +3254,24 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
OS << " }\n";
OS << "\n";
OS << " Inst.clear();\n\n";
OS << " Inst.setOpcode(it->Opcode);\n";
// Verify the instruction with the target-specific match predicate function.
OS << " // We have a potential match but have not rendered the operands.\n"
<< " // Check the target predicate to handle any context sensitive\n"
" // constraints.\n"
<< " // For example, Ties that are referenced multiple times must be\n"
" // checked here to ensure the input is the same for each match\n"
" // constraints. If we leave it any later the ties will have been\n"
" // canonicalized\n"
<< " unsigned MatchResult;\n"
<< " if ((MatchResult = checkEarlyTargetMatchPredicate(Inst, "
"Operands)) != Match_Success) {\n"
<< " Inst.clear();\n"
<< " RetCode = MatchResult;\n"
<< " HadMatchOtherThanPredicate = true;\n"
<< " continue;\n"
<< " }\n\n";
OS << " if (matchingInlineAsm) {\n";
OS << " Inst.setOpcode(it->Opcode);\n";
OS << " convertToMapAndConstraints(it->ConvertFn, Operands);\n";
OS << " return Match_Success;\n";
OS << " }\n\n";
@ -3272,7 +3288,6 @@ void AsmMatcherEmitter::run(raw_ostream &OS) {
// Verify the instruction with the target-specific match predicate function.
OS << " // We have a potential match. Check the target predicate to\n"
<< " // handle any context sensitive constraints.\n"
<< " unsigned MatchResult;\n"
<< " if ((MatchResult = checkTargetMatchPredicate(Inst)) !="
<< " Match_Success) {\n"
<< " Inst.clear();\n"