Skip to content

Commit

Permalink
[MIPS] Fix useDeprecatedPositionallyEncodedOperands errors.
Browse files Browse the repository at this point in the history 
This is a follow-on to https://reviews.llvm.org/D134073.

The number of MIPS16 changes here is a bit surprising. Many of the
fields with mismatched names were NOT previously choosing the correct
argument positionally, but instead doing something completely wrong
(e.g. it would encode a register where an immediate was expected).

But, machine-code generation for MIPS16 has never actually functioned.
It's also fully untested, thus, the MIPS16 changes, despite changing
behavior, breaks (and fixes) zero tests. This change does not fix
MIPS16 output, but it ought to be at least incrementally less broken.

Outside MIPS16, I believe the only functional change is to the 'ginvi'
instruction: it was previously encoding garbage into a field which was
specified to be '00'. Fortunately, it was covered by tests -- and the
tests were testing the incorrect behavior. So, fixed.

Differential Revision: https://reviews.llvm.org/D134220
  • Loading branch information
@jyknight
jyknight committed Oct 26, 2022
1 parent 23394cd commit 26fdad0
Show file tree
Hide file tree
Showing 13 changed files with 66 additions and 56 deletions.
5 changes: 3 additions & 2 deletions llvm/lib/Target/Mips/MicroMips32r6InstrFormats.td
View file
Expand Up @@ -733,14 +733,15 @@ class POOL16C_NOT16_FM_MMR6 {
}

class POOL16C_MOVEP16_FM_MMR6 {
bits<3> dst_regs;
bits<3> rt;
bits<3> rs;

bits<16> Inst;

let Inst{15-10} = 0b010001;
let Inst{9-7} = dst_regs;
// bits 7-9 are populated by MipsMCCodeEmitter::encodeInstruction, with a
// special encoding of both rd1 and rd2.
let Inst{9-7} = ?;
let Inst{6-4} = rt;
let Inst{3} = rs{2};
let Inst{2} = 0b1;
Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/Mips/MicroMips32r6InstrInfo.td
View file
Expand Up @@ -822,6 +822,7 @@ class GINV_MMR6_DESC_BASE<string opstr,

class GINVI_MMR6_DESC : GINV_MMR6_DESC_BASE<"ginvi", GPR32Opnd,
II_GINVI> {
bits<2> type = 0b00;
dag InOperandList = (ins GPR32Opnd:$rs);
string AsmString = "ginvi\t$rs";
}
Expand Down
5 changes: 3 additions & 2 deletions llvm/lib/Target/Mips/MicroMipsInstrFormats.td
View file
Expand Up @@ -271,14 +271,15 @@ class B16_FM {
}

class MOVEP_FM_MM16 {
bits<3> dst_regs;
bits<3> rt;
bits<3> rs;

bits<16> Inst;

let Inst{15-10} = 0x21;
let Inst{9-7} = dst_regs;
// bits 7-9 are populated by MipsMCCodeEmitter::encodeInstruction, with a
// special encoding of both rd1 and rd2.
let Inst{9-7} = ?;
let Inst{6-4} = rt;
let Inst{3-1} = rs;
let Inst{0} = 0;
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Target/Mips/Mips.td
View file
Expand Up @@ -224,7 +224,6 @@ include "MipsScheduleP5600.td"
include "MipsScheduleGeneric.td"

def MipsInstrInfo : InstrInfo {
let useDeprecatedPositionallyEncodedOperands = 1;
}

//===----------------------------------------------------------------------===//
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Target/Mips/Mips16InstrFormats.td
View file
Expand Up @@ -336,7 +336,7 @@ class FI8_MOVR3216<dag outs, dag ins, string asmstr,
list<dag> pattern, InstrItinClass itin>:
MipsInst16<outs, ins, asmstr, pattern, itin>
{

// FIXME: this seems wrong? 'ry' should be 3 bits, and 'r32' 5?
bits<4> ry;
bits<4> r32;

Expand Down
91 changes: 47 additions & 44 deletions llvm/lib/Target/Mips/Mips16InstrInfo.td
View file
Expand Up @@ -51,8 +51,8 @@ def pcrel16 : Operand<i32>;
// number
//
class FI16_ins<bits<5> op, string asmstr, InstrItinClass itin>:
FI16<op, (outs), (ins brtarget:$imm16),
!strconcat(asmstr, "\t$imm16 # 16 bit inst"), [], itin>;
FI16<op, (outs), (ins brtarget:$imm11),
!strconcat(asmstr, "\t$imm11 # 16 bit inst"), [], itin>;

//
//
Expand All @@ -61,16 +61,16 @@ class FI16_ins<bits<5> op, string asmstr, InstrItinClass itin>:

class FI816_ins_base<bits<3> _func, string asmstr,
string asmstr2, InstrItinClass itin>:
FI816<_func, (outs), (ins simm16:$imm), !strconcat(asmstr, asmstr2),
FI816<_func, (outs), (ins simm16:$imm8), !strconcat(asmstr, asmstr2),
[], itin>;

class FI816_ins<bits<3> _func, string asmstr,
InstrItinClass itin>:
FI816_ins_base<_func, asmstr, "\t$imm # 16 bit inst", itin>;
FI816_ins_base<_func, asmstr, "\t$imm8 # 16 bit inst", itin>;

class FI816_SP_ins<bits<3> _func, string asmstr,
InstrItinClass itin>:
FI816_ins_base<_func, asmstr, "\t$$sp, $imm # 16 bit inst", itin>;
FI816_ins_base<_func, asmstr, "\t$$sp, $imm8 # 16 bit inst", itin>;

//
// RI instruction format
Expand All @@ -79,38 +79,38 @@ class FI816_SP_ins<bits<3> _func, string asmstr,

class FRI16_ins_base<bits<5> op, string asmstr, string asmstr2,
InstrItinClass itin>:
FRI16<op, (outs CPU16Regs:$rx), (ins simm16:$imm),
FRI16<op, (outs CPU16Regs:$rx), (ins simm16:$imm8),
!strconcat(asmstr, asmstr2), [], itin>;

class FRI16_ins<bits<5> op, string asmstr,
InstrItinClass itin>:
FRI16_ins_base<op, asmstr, "\t$rx, $imm \t# 16 bit inst", itin>;
FRI16_ins_base<op, asmstr, "\t$rx, $imm8 \t# 16 bit inst", itin>;

class FRI16_TCP_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FRI16<_op, (outs CPU16Regs:$rx), (ins pcrel16:$imm, i32imm:$size),
!strconcat(asmstr, "\t$rx, $imm\t# 16 bit inst"), [], itin>;
FRI16<_op, (outs CPU16Regs:$rx), (ins pcrel16:$imm8, i32imm:$size),
!strconcat(asmstr, "\t$rx, $imm8\t# 16 bit inst"), [], itin>;

class FRI16R_ins_base<bits<5> op, string asmstr, string asmstr2,
InstrItinClass itin>:
FRI16<op, (outs), (ins CPU16Regs:$rx, simm16:$imm),
FRI16<op, (outs), (ins CPU16Regs:$rx, simm16:$imm8),
!strconcat(asmstr, asmstr2), [], itin>;

class FRI16R_ins<bits<5> op, string asmstr,
InstrItinClass itin>:
FRI16R_ins_base<op, asmstr, "\t$rx, $imm \t# 16 bit inst", itin>;
FRI16R_ins_base<op, asmstr, "\t$rx, $imm8 \t# 16 bit inst", itin>;

class F2RI16_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FRI16<_op, (outs CPU16Regs:$rx), (ins CPU16Regs:$rx_, simm16:$imm),
!strconcat(asmstr, "\t$rx, $imm\t# 16 bit inst"), [], itin> {
FRI16<_op, (outs CPU16Regs:$rx), (ins CPU16Regs:$rx_, simm16:$imm8),
!strconcat(asmstr, "\t$rx, $imm8\t# 16 bit inst"), [], itin> {
let Constraints = "$rx_ = $rx";
}

class FRI16_B_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FRI16<_op, (outs), (ins CPU16Regs:$rx, brtarget:$imm),
!strconcat(asmstr, "\t$rx, $imm # 16 bit inst"), [], itin>;
FRI16<_op, (outs), (ins CPU16Regs:$rx, brtarget:$imm8),
!strconcat(asmstr, "\t$rx, $imm8 # 16 bit inst"), [], itin>;
//
// Compare a register and immediate and place result in CC
// Implicit use of T8
Expand All @@ -128,17 +128,17 @@ class FEXT_CCRXI16_ins<string asmstr>:
//
class FJAL16_ins<bits<1> _X, string asmstr,
InstrItinClass itin>:
FJAL16<_X, (outs), (ins uimm26:$imm),
!strconcat(asmstr, "\t$imm\n\tnop"),[],
FJAL16<_X, (outs), (ins uimm26:$imm26),
!strconcat(asmstr, "\t$imm26\n\tnop"),[],
itin> {
let isCodeGenOnly=1;
let Size=6;
}

class FJALB16_ins<bits<1> _X, string asmstr,
InstrItinClass itin>:
FJAL16<_X, (outs), (ins uimm26:$imm),
!strconcat(asmstr, "\t$imm\t# branch\n\tnop"),[],
FJAL16<_X, (outs), (ins uimm26:$imm26),
!strconcat(asmstr, "\t$imm26\t# branch\n\tnop"),[],
itin> {
let isCodeGenOnly=1;
let Size=6;
Expand All @@ -157,16 +157,16 @@ class FEXT_I16_ins<bits<5> eop, string asmstr, InstrItinClass itin> :

class FEXT_I816_ins_base<bits<3> _func, string asmstr,
string asmstr2, InstrItinClass itin>:
FEXT_I816<_func, (outs), (ins simm16:$imm), !strconcat(asmstr, asmstr2),
FEXT_I816<_func, (outs), (ins simm16:$imm16), !strconcat(asmstr, asmstr2),
[], itin>;

class FEXT_I816_ins<bits<3> _func, string asmstr,
InstrItinClass itin>:
FEXT_I816_ins_base<_func, asmstr, "\t$imm", itin>;
FEXT_I816_ins_base<_func, asmstr, "\t$imm16", itin>;

class FEXT_I816_SP_ins<bits<3> _func, string asmstr,
InstrItinClass itin>:
FEXT_I816_ins_base<_func, asmstr, "\t$$sp, $imm", itin>;
FEXT_I816_ins_base<_func, asmstr, "\t$$sp, $imm16", itin>;

//
// Assembler formats in alphabetical order.
Expand All @@ -190,39 +190,39 @@ class FCCRR16_ins<string asmstr> :

class FEXT_RI16_ins_base<bits<5> _op, string asmstr, string asmstr2,
InstrItinClass itin>:
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins simm16:$imm),
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins simm16:$imm16),
!strconcat(asmstr, asmstr2), [], itin>;

class FEXT_RI16_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FEXT_RI16_ins_base<_op, asmstr, "\t$rx, $imm", itin>;
FEXT_RI16_ins_base<_op, asmstr, "\t$rx, $imm16", itin>;

class FEXT_RI16R_ins_base<bits<5> _op, string asmstr, string asmstr2,
InstrItinClass itin>:
FEXT_RI16<_op, (outs ), (ins CPU16Regs:$rx, simm16:$imm),
FEXT_RI16<_op, (outs ), (ins CPU16Regs:$rx, simm16:$imm16),
!strconcat(asmstr, asmstr2), [], itin>;

class FEXT_RI16R_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FEXT_RI16R_ins_base<_op, asmstr, "\t$rx, $imm", itin>;
FEXT_RI16R_ins_base<_op, asmstr, "\t$rx, $imm16", itin>;

class FEXT_RI16_PC_ins<bits<5> _op, string asmstr, InstrItinClass itin>:
FEXT_RI16_ins_base<_op, asmstr, "\t$rx, $$pc, $imm", itin>;
FEXT_RI16_ins_base<_op, asmstr, "\t$rx, $$pc, $imm16", itin>;

class FEXT_RI16_B_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FEXT_RI16<_op, (outs), (ins CPU16Regs:$rx, brtarget:$imm),
!strconcat(asmstr, "\t$rx, $imm"), [], itin>;
FEXT_RI16<_op, (outs), (ins CPU16Regs:$rx, brtarget:$imm16),
!strconcat(asmstr, "\t$rx, $imm16"), [], itin>;

class FEXT_RI16_TCP_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins pcrel16:$imm, i32imm:$size),
!strconcat(asmstr, "\t$rx, $imm"), [], itin>;
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins pcrel16:$imm16, i32imm:$size),
!strconcat(asmstr, "\t$rx, $imm16"), [], itin>;

class FEXT_2RI16_ins<bits<5> _op, string asmstr,
InstrItinClass itin>:
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins CPU16Regs:$rx_, simm16:$imm),
!strconcat(asmstr, "\t$rx, $imm"), [], itin> {
FEXT_RI16<_op, (outs CPU16Regs:$rx), (ins CPU16Regs:$rx_, simm16:$imm16),
!strconcat(asmstr, "\t$rx, $imm16"), [], itin> {
let Constraints = "$rx_ = $rx";
}

Expand All @@ -232,12 +232,12 @@ class FEXT_2RI16_ins<bits<5> _op, string asmstr,

class FEXT_RRI16_mem_ins<bits<5> op, string asmstr, Operand MemOpnd,
InstrItinClass itin>:
FEXT_RRI16<op, (outs CPU16Regs:$ry), (ins MemOpnd:$addr),
FEXT_RRI16<op, (outs CPU16Regs:$ry), (ins (MemOpnd $rx, $imm16):$addr),
!strconcat(asmstr, "\t$ry, $addr"), [], itin>;

class FEXT_RRI16_mem2_ins<bits<5> op, string asmstr, Operand MemOpnd,
InstrItinClass itin>:
FEXT_RRI16<op, (outs ), (ins CPU16Regs:$ry, MemOpnd:$addr),
FEXT_RRI16<op, (outs ), (ins CPU16Regs:$ry, (MemOpnd $rx, $imm16):$addr),
!strconcat(asmstr, "\t$ry, $addr"), [], itin>;

//
Expand All @@ -247,15 +247,15 @@ class FEXT_RRI16_mem2_ins<bits<5> op, string asmstr, Operand MemOpnd,

class FEXT_RRI_A16_mem_ins<bits<1> op, string asmstr, Operand MemOpnd,
InstrItinClass itin>:
FEXT_RRI_A16<op, (outs CPU16Regs:$ry), (ins MemOpnd:$addr),
FEXT_RRI_A16<op, (outs CPU16Regs:$ry), (ins (MemOpnd $rx, $imm15):$addr),
!strconcat(asmstr, "\t$ry, $addr"), [], itin>;

//
// EXT-SHIFT instruction format
//
class FEXT_SHIFT16_ins<bits<2> _f, string asmstr, InstrItinClass itin>:
FEXT_SHIFT16<_f, (outs CPU16Regs:$rx), (ins CPU16Regs:$ry, uimm5:$sa),
!strconcat(asmstr, "\t$rx, $ry, $sa"), [], itin>;
FEXT_SHIFT16<_f, (outs CPU16Regs:$rx), (ins CPU16Regs:$ry, uimm5:$sa6),
!strconcat(asmstr, "\t$rx, $ry, $sa6"), [], itin>;

//
// EXT-T8I8
Expand Down Expand Up @@ -287,8 +287,8 @@ class FEXT_T8I8I16_ins<string asmstr, string asmstr2>:
// I8_MOVR32 instruction format (used only by the MOVR32 instructio
//
class FI8_MOVR3216_ins<string asmstr, InstrItinClass itin>:
FI8_MOVR3216<(outs CPU16Regs:$rz), (ins GPR32:$r32),
!strconcat(asmstr, "\t$rz, $r32"), [], itin>;
FI8_MOVR3216<(outs CPU16Regs:$ry), (ins GPR32:$r32),
!strconcat(asmstr, "\t$ry, $r32"), [], itin>;

//
// I8_MOV32R instruction format (used only by MOV32R instruction)
Expand Down Expand Up @@ -374,8 +374,8 @@ class FRR16_JALRC_RA_only_ins<bits<1> nd_, bits<1> l_,

class FRR16_JALRC_ins<bits<1> nd, bits<1> l, bits<1> ra,
string asmstr, InstrItinClass itin>:
FRR16_JALRC<nd, l, ra, (outs), (ins CPU16Regs:$rx),
!strconcat(asmstr, "\t$rx"), [], itin> ;
FRR16_JALRC<nd, l, ra, (outs), (ins CPU16Regs:$rs),
!strconcat(asmstr, "\t$rs"), [], itin> ;

class FRR_SF16_ins
<bits<5> _funct, bits<3> _subfunc,
Expand Down Expand Up @@ -775,6 +775,7 @@ def JrcRa16: FRR16_JALRC_RA_only_ins<1, 1, "jrc", IIM16Alu> {
}

def JrcRx16: FRR16_JALRC_ins<1, 1, 0, "jrc", IIM16Alu> {
let rx = 0b000;
let isBranch = 1;
let isIndirectBranch = 1;
let isTerminator=1;
Expand Down Expand Up @@ -876,6 +877,7 @@ def MoveR3216: FI8_MOVR3216_ins<"move", IIM16Alu> {
// To copy the special purpose HI register to a GPR.
//
def Mfhi16: FRR16_M_ins<0b10000, "mfhi", IIM16Alu> {
let ry = 0b000; // no 'ry' field
let Uses = [HI0];
let hasSideEffects = 0;
let isMoveReg = 1;
Expand All @@ -887,6 +889,7 @@ def Mfhi16: FRR16_M_ins<0b10000, "mfhi", IIM16Alu> {
// To copy the special purpose LO register to a GPR.
//
def Mflo16: FRR16_M_ins<0b10010, "mflo", IIM16Alu> {
let ry = 0b000; // no 'ry' field
let Uses = [LO0];
let hasSideEffects = 0;
let isMoveReg = 0;
Expand Down Expand Up @@ -1376,8 +1379,8 @@ def: Mips16Pat<(brind CPU16Regs:$rs), (JrcRx16 CPU16Regs:$rs)> {
// Jump and Link (Call)
let isCall=1, hasDelaySlot=0 in
def JumpLinkReg16:
FRR16_JALRC<0, 0, 0, (outs), (ins CPU16Regs:$rs),
"jalrc\t$rs", [(MipsJmpLink CPU16Regs:$rs)], II_JALRC> {
FRR16_JALRC<0, 0, 0, (outs), (ins CPU16Regs:$rx),
"jalrc\t$rx", [(MipsJmpLink CPU16Regs:$rx)], II_JALRC> {
let Defs = [RA];
}

Expand Down
1 change: 1 addition & 0 deletions llvm/lib/Target/Mips/Mips32r6InstrInfo.td
View file
Expand Up @@ -841,6 +841,7 @@ class GINV_DESC_BASE<string instr_asm, RegisterOperand GPROpnd,
}

class GINVI_DESC : GINV_DESC_BASE<"ginvi", GPR32Opnd, II_GINVI> {
bits<2> type_ = 0b00;
dag InOperandList = (ins GPR32Opnd:$rs);
string AsmString = "ginvi\t$rs";
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/Target/Mips/MipsEVAInstrFormats.td
View file
Expand Up @@ -59,15 +59,15 @@ def OPGROUP_COP0_TLB : OPGROUP<0b010000>;

class SPECIAL3_EVA_LOAD_STORE_FM<OPCODE6 Operation> : MipsEVAInst {
bits<21> addr;
bits<5> hint;
bits<5> rt;
bits<5> base = addr{20-16};
bits<9> offset = addr{8-0};

bits<32> Inst;

let Inst{31-26} = OPGROUP_SPECIAL3.Value;
let Inst{25-21} = base;
let Inst{20-16} = hint;
let Inst{20-16} = rt;
let Inst{15-7} = offset;
let Inst{6} = 0;
let Inst{5-0} = Operation.Value;
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/Mips/MipsEVAInstrInfo.td
View file
Expand Up @@ -163,6 +163,10 @@ class TLBINVF_DESC : TLB_DESC_BASE<"tlbinvf", II_TLBINVF>;

class CACHEE_DESC_BASE<string instr_asm, Operand MemOpnd,
InstrItinClass itin = NoItinerary> {
// CACHEE puts the "hint" immediate where the encoding would otherwise have "rt"
bits<5> hint;
bits<5> rt = hint;

dag OutOperandList = (outs);
dag InOperandList = (ins MemOpnd:$addr, uimm5:$hint);
string AsmString = !strconcat(instr_asm, "\t$hint, $addr");
Expand Down
2 changes: 1 addition & 1 deletion llvm/test/MC/Disassembler/Mips/ginv/valid-el.txt
View file
@@ -1,5 +1,5 @@
# RUN: llvm-mc --disassemble %s -triple=mipsel-unknown-linux-gnu \
# RUN: -mcpu=mips32r6 -mattr=+ginv | FileCheck %s

0x3d 0x02 0x40 0x7c # CHECK: ginvi $2
0x3d 0x00 0x40 0x7c # CHECK: ginvi $2
0xbd 0x02 0x40 0x7c # CHECK: ginvt $2, 2
2 changes: 1 addition & 1 deletion llvm/test/MC/Disassembler/Mips/ginv/valid-micromips-el.txt
View file
@@ -1,5 +1,5 @@
# RUN: llvm-mc --disassemble %s -triple=mipsel-unknown-linux-gnu \
# RUN: -mcpu=mips32r6 -mattr=+micromips,+ginv | FileCheck %s

0x02 0x00 0x7c 0x65 # CHECK: ginvi $2
0x02 0x00 0x7c 0x61 # CHECK: ginvi $2
0x02 0x00 0x7c 0x75 # CHECK: ginvt $2, 2
2 changes: 1 addition & 1 deletion llvm/test/MC/Disassembler/Mips/ginv/valid-micromips.txt
View file
@@ -1,5 +1,5 @@
# RUN: llvm-mc --disassemble %s -triple=mips-unknown-linux-gnu \
# RUN: -mcpu=mips32r6 -mattr=+micromips,+ginv | FileCheck %s

0x00 0x02 0x65 0x7c # CHECK: ginvi $2
0x00 0x02 0x61 0x7c # CHECK: ginvi $2
0x00 0x02 0x75 0x7c # CHECK: ginvt $2, 2
2 changes: 1 addition & 1 deletion llvm/test/MC/Disassembler/Mips/ginv/valid.txt
View file
@@ -1,5 +1,5 @@
# RUN: llvm-mc --disassemble %s -triple=mips-unknown-linux-gnu \
# RUN: -mcpu=mips32r6 -mattr=+ginv | FileCheck %s

0x7c 0x40 0x02 0x3d # CHECK: ginvi $2
0x7c 0x40 0x00 0x3d # CHECK: ginvi $2
0x7c 0x40 0x02 0xbd # CHECK: ginvt $2, 2

0 comments on commit 26fdad0

Please sign in to comment.