[PATCH] D11178: [mips][micromips] Implement ADDU16, AND16, ANDI16, NOT16, OR16, SLL16 and SRL16 instructions

[Daniel Sanders via llvm-commits]

dsanders requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1718-1720
@@ +1717,5 @@
+static DecodeStatus DecodePOOL16BEncodedField(MCInst &Inst,
+                                  unsigned Value,
+                                  uint64_t Address,
+                                  const void *Decoder) {
+  if (Value == 0x0)
----------------
Nit: Indentation

================
Comment at: lib/Target/Mips/Disassembler/MipsDisassembler.cpp:1721-1724
@@ +1720,6 @@
+                                  const void *Decoder) {
+  if (Value == 0x0)
+    Inst.addOperand(MCOperand::createImm(8));
+  else
+    Inst.addOperand(MCOperand::createImm(Value));
+  return MCDisassembler::Success;
----------------
Nit: Use ternary operator

================
Comment at: lib/Target/Mips/MicroMips32r6InstrFormats.td:294
@@ +293,3 @@
+  bits<3> rt;
+  bits<3> rd;
+
----------------
Style nit: rd is normally first.

================
Comment at: lib/Target/Mips/MicroMips32r6InstrInfo.td:287-291
@@ -279,1 +286,7 @@
 
+// Class used for microMIPS32r6 and microMIPS64r6 instructions.
+class MicroMipsR6Inst16 {
+  string DecoderNamespace = "MicroMipsR6";
+  list<Predicate> Predicates = [HasStdEnc, HasMicroMips32r6];
+}
+
----------------
There's several issues here:
* This class belongs in MicroMips32r6InstrFormats.td similar to MipsR6Inst in Mips32r6InstrFormats.td and the formats (*_FM) should inherit from it, not the descriptions (*_DESC)
* Don't use Predicates directly. Inherit from PredicateControl and use its fields.
* DecoderNamespace is a property of the format/encoding and not of the description.
* HasStdEnc is a predicate of the format/encoding and belongs in the EncodingPredicates member of the format, but ...
  * ... HasStdEnc means not microMIPS and not MIPS16. It's not correct to use it for microMIPS.
* Use ISA_MICROMIPS32R6 on the def since it's a property of the instruction and not the description. Also it's more consistent with the other instructions.
  * I see you've already have ISA_MICROMIPS32R6 on the instructions. The reason the adjective isn't working there is because you aren't using PredicateControl and/or you overrode Predicates.


http://reviews.llvm.org/D11178