[PATCH] D44905: [mips] Add support for Virtualization ASE

[Simon Dardis via Phabricator via llvm-commits]

sdardis added a comment.

Only some minor nits now, inlined.



================
Comment at: lib/Target/Mips/MicroMipsInstrInfo.td:1280-1293
+  def : MipsInstAlias<"hypcall", (HYPCALL_MM 0), 1>,
+                      ISA_MICROMIPS32R5, ASE_VIRT;
+  def : MipsInstAlias<"mfgc0 $rt, $rs",
+                      (MFGC0_MM GPR32Opnd:$rt, COP0Opnd:$rs, 0), 0>,
+                      ISA_MICROMIPS32R5, ASE_VIRT;
+  def : MipsInstAlias<"mfhgc0 $rt, $rs",
+                      (MFHGC0_MM GPR32Opnd:$rt, COP0Opnd:$rs, 0), 0>,
----------------
Deindent these, they should be aligned to the closing brace above them since they're not a part of the scope the the brace defines.


================
Comment at: lib/Target/Mips/Mips64InstrInfo.td:562
+               MFC3OP_FM<0x10, 3, 1>, ISA_MIPS64R5, ASE_VIRT;
+  def DMTGC0 : MFC3OP<"dmtgc0", COP0Opnd, GPR64Opnd, II_DMTGC0>,
+               MFC3OP_FM<0x10, 3, 3>, ISA_MIPS64R5, ASE_VIRT;
----------------
This should be MTC3OP<...>.


================
Comment at: lib/Target/Mips/MipsInstrInfo.td:2715
 
+let AdditionalPredicates = [NotInMicroMips] in {
+  def : MipsInstAlias<"hypcall", (HYPCALL 0), 1>, ISA_MIPS32R5, ASE_VIRT;
----------------
You can drop the braces here so the let clause only applies to the next definition/class.


================
Comment at: test/MC/Mips/virt/valid64.s:6
+  dmfgc0 $4,$5,4    # CHECK: dmfgc0 $4, $5, 4  # encoding: [0x40,0x64,0x29,0x04]
+  dmtgc0 $4,$5      # CHECK: dmtgc0 $5, $4, 0  # encoding: [0x40,0x65,0x23,0x00]
+  dmtgc0 $4,$5,4    # CHECK: dmtgc0 $4, $5, 4  # encoding: [0x40,0x64,0x2b,0x04]
----------------
Once you fix the class definition of dmtgc0, this line will fail.


Repository:
  rL LLVM

https://reviews.llvm.org/D44905