[PATCH] D119665: [M68k] Adopt VarLenCodeEmitter for control instructions

[Min-Yih Hsu via Phabricator via llvm-commits]

myhsu added a comment.

Thanks for the patch :-)
I only have minor comments (and please remember to XFAIL control.txt per my previous comment)



================
Comment at: llvm/lib/Target/M68k/M68kInstrControl.td:169
+        (descend 0b0110, !cast<MxEncCondOp>("MxCC"#cc).Value, disp_8),
+        disp_16_32
+      );
----------------
I'm fine with the syntax here, however in the future if you want to avoid supplying `disp_16_32` with empty `(ascend)` for 16/32 bits variants, you can use the conditional bang operator `!cond` like this:
```
class MxBcc<string cc, Operand TARGET, int SIZE>... {
  ...
  let Inst = !cond(
      !eq(SIZE, 8):   /*encoding for Bcc8*/,
      !eq(SIZE, 16):  /*encoding for Bcc16*/,
      !eq(SIZE, 32):  /*encoding for Bcc32*/
  );
}
```


================
Comment at: llvm/lib/Target/M68k/M68kInstrControl.td:176-177
   def B#cc#"8"
     : MxBcc<cc, MxBrTarget8,
-            MxEncoding<MxBead8Disp<0>,
-                       !cast<MxBead4Bits>("MxCC"#cc), MxBead4Bits<0x6>>>;
+        (operand "$dst", 8, (encoder "encodePCRelImm<8>")), (ascend)>;
+
----------------
nit formatting


================
Comment at: llvm/lib/Target/M68k/M68kInstrControl.td:180-181
   def B#cc#"16"
-    : MxBcc<cc, MxBrTarget16,
-            MxEncoding<MxBead4Bits<0x0>,
-                       MxBead4Bits<0x0>, !cast<MxBead4Bits>("MxCC"#cc),
-                       MxBead4Bits<0x6>, MxBead16Imm<0>>>;
+    : MxBcc<cc, MxBrTarget16, (descend 0b0000, 0b0000),
+        (operand "$dst", 16, (encoder "encodePCRelImm<16>"))>;
 }
----------------



================
Comment at: llvm/lib/Target/M68k/M68kInstrControl.td:209-210
 
 def BRA8  : MxBra<MxBrTarget8,
-                  MxEncoding<MxBead8Disp<0>, MxBead4Bits<0x0>,
-                             MxBead4Bits<0x6>>>;
-def BRA16 : MxBra<MxBrTarget16,
-                  MxEncoding<MxBead4Bits<0x0>, MxBead4Bits<0x0>,
-                             MxBead4Bits<0x0>, MxBead4Bits<0x6>,
-                             MxBead16Imm<0>>>;
+                (operand "$dst", 8, (encoder "encodePCRelImm<8>")), (ascend)>;
+
----------------



================
Comment at: llvm/lib/Target/M68k/M68kInstrControl.td:212-213
+
+def BRA16 : MxBra<MxBrTarget16, (descend 0b0000, 0b0000),
+                (operand "$dst", 16, (encoder "encodePCRelImm<16>"))>;
 
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119665/new/

https://reviews.llvm.org/D119665