[M68k] Adopt VarLenCodeEmitter for control instructions

[0x59616e]

Summary

Refactor the instructions in M68kInstrControl.td to use the VarLenCodeEmitter.

This patch is tested by the existing test cases.

[0x59616e]

update diff

[ricky26]

Thanks for this, I'm really enjoying the new syntax, it's so much easier to follow. I'm a little bit unsure about the test change.

llvm/lib/Target/M68k/M68kInstrControl.td

47

nit: Any reason you've split this up by byte? Generally the manuals I've read have it as one string of bits (I wonder if the bit syntax supports underscores).

llvm/test/MC/M68k/Control/call-pc-rel.s

3 ↗

(On Diff #408301)

It looks like this just negates the whole test?

[0x59616e]

llvm/lib/Target/M68k/M68kInstrControl.td

47

Just for readability.

And...no, the bit syntax does not support underscore.

llvm/test/MC/M68k/Control/call-pc-rel.s

3 ↗

(On Diff #408301)

This just negates the second test, which needs disassembler support.

The first test is still online.

[ricky26]

llvm/test/MC/M68k/Control/call-pc-rel.s

3 ↗

(On Diff #408301)

Is it worth adding a TODO: here? I don't want it to get lost and I don't think it would be immediately obvious to me that it was disabled.

[0x59616e]

llvm/test/MC/M68k/Control/call-pc-rel.s

3 ↗

(On Diff #408301)

You have a point. Thanks ;)

[0x59616e]

address feedback

[ricky26]

LGTM. Might be worth waiting for @myhsu to review, but I don't think there's anything controversial. Thanks! 😄

[myhsu]

Please also XFAIL test/MC/Disassembler/M68k/control.txt

[myhsu]

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

llvm/lib/Target/M68k/M68kInstrControl.td

164

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*/
  );
}

171–173

nit formatting

175–177

198–204

206–207

[0x59616e]

address feedback

[0x59616e]

llvm/lib/Target/M68k/M68kInstrControl.td

164

I leave a FIXME here ;)

[myhsu]

LGTM
please address my last comment before commit though

llvm/lib/Target/M68k/M68kInstrControl.td

162

"we can we conditional"?

[0x59616e]

address feedback

[0x59616e]

llvm/lib/Target/M68k/M68kInstrControl.td

162

my bad

[0x59616e]

rebase

[0x59616e]

OK I think all the feedbacks are addressed properly.

Thanks @ricky26 @myhsu ;)

llvm/lib/Target/M68k/M68kInstrControl.td

 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // NOP
 //===----------------------------------------------------------------------===//
 
 let hasSideEffects = 0 in {
   def NOP : MxInst<(outs), (ins), "nop", [], MxEncFixed<0x4E71>>;
 }
 
 
 //===----------------------------------------------------------------------===//
 // Conditions
 //===----------------------------------------------------------------------===//
 
 /// CC—Carry clear      GE—Greater than or equal
 /// LS—Lower or same    PL—Plus
 /// CS—Carry set        GT—Greater than
 /// LT—Less than        T—Always true*
 /// EQ—Equal            HI—Higher
 /// MI—Minus            VC—Overflow clear
 /// F—Never true*       LE—Less than or equal
 /// NE—Not equal        VS—Overflow set
 ///
 /// *Not applicable to the Bcc instructions.
-def MxCCt  : MxBead4Bits<0b0000>;
-def MxCCf  : MxBead4Bits<0b0001>;
-def MxCChi : MxBead4Bits<0b0010>;
-def MxCCls : MxBead4Bits<0b0011>;
-def MxCCcc : MxBead4Bits<0b0100>;
-def MxCCcs : MxBead4Bits<0b0101>;
-def MxCCne : MxBead4Bits<0b0110>;
-def MxCCeq : MxBead4Bits<0b0111>;
-def MxCCvc : MxBead4Bits<0b1000>;
-def MxCCvs : MxBead4Bits<0b1001>;
-def MxCCpl : MxBead4Bits<0b1010>;
-def MxCCmi : MxBead4Bits<0b1011>;
-def MxCCge : MxBead4Bits<0b1100>;
-def MxCClt : MxBead4Bits<0b1101>;
-def MxCCgt : MxBead4Bits<0b1110>;
-def MxCCle : MxBead4Bits<0b1111>;
+class MxEncCondOp<bits<4> cond> {
+  dag Value = (descend cond);
+}
+
+def MxCCt  : MxEncCondOp<0b0000>;
+def MxCCf  : MxEncCondOp<0b0001>;
+def MxCChi : MxEncCondOp<0b0010>;
+def MxCCls : MxEncCondOp<0b0011>;
+def MxCCcc : MxEncCondOp<0b0100>;
+def MxCCcs : MxEncCondOp<0b0101>;
+def MxCCne : MxEncCondOp<0b0110>;
+def MxCCeq : MxEncCondOp<0b0111>;
+def MxCCvc : MxEncCondOp<0b1000>;
+def MxCCvs : MxEncCondOp<0b1001>;
+def MxCCpl : MxEncCondOp<0b1010>;
+def MxCCmi : MxEncCondOp<0b1011>;
+def MxCCge : MxEncCondOp<0b1100>;
+def MxCClt : MxEncCondOp<0b1101>;
+def MxCCgt : MxEncCondOp<0b1110>;
+def MxCCle : MxEncCondOp<0b1111>;
+
+
 
 /// --------------------------------+---------+---------
 ///  F  E  D  C | B  A  9  8 | 7  6 | 5  4  3 | 2  1  0
 /// --------------------------------+---------+---------
 ///  0  1  0  1 | CONDITION  | 1  1 |   MODE  |   REG
 /// ----------------------------------------------------
-class MxSccEncoding<MxEncEA EA, MxEncExt EXT, MxBead4Bits CC>
-    : MxEncoding<EA.Reg, EA.DA, EA.Mode, MxBead2Bits<0b11>, CC, MxBead4Bits<0b0101>,
-                 EXT.Imm, EXT.B8, EXT.Scale, EXT.WL, EXT.DAReg>;
 
 let Uses = [CCR] in {
 class MxSccR<string CC>
     : MxInst<(outs MxDRD8:$dst), (ins), "s"#CC#"\t$dst",
-             [(set i8:$dst, (MxSetCC !cast<PatLeaf>("MxCOND"#CC), CCR))],
-             MxSccEncoding<MxEncEAd_0, MxExtEmpty,
-                           !cast<MxBead4Bits>("MxCC"#CC)>>;
+             [(set i8:$dst, (MxSetCC !cast<PatLeaf>("MxCOND"#CC), CCR))]> {
+  let Inst = (descend 0b0101, !cast<MxEncCondOp>("MxCC"#CC).Value, 0b11,
+              /*MODE without last bit*/0b00,
+              /*REGISTER prefixed with D/A bit*/(operand "$dst", 4));
+}
 
-class MxSccM<string CC, MxOperand MEMOpd, ComplexPattern MEMPat,
-             MxEncEA EA, MxEncExt EXT>
+class MxSccM<string CC, MxOperand MEMOpd, ComplexPattern MEMPat, MxEncMemOp DST_ENC>
     : MxInst<(outs), (ins MEMOpd:$dst), "s"#CC#"\t$dst",
-             [(store (MxSetCC !cast<PatLeaf>("MxCOND"#CC), CCR), MEMPat:$dst)],
-             MxSccEncoding<EA, EXT, !cast<MxBead4Bits>("MxCC"#CC)>>;
+             [(store (MxSetCC !cast<PatLeaf>("MxCOND"#CC), CCR), MEMPat:$dst)]> {
+  let Inst =
+    (ascend
+      (descend 0b0101, !cast<MxEncCondOp>("MxCC"#CC).Value, 0b11, DST_ENC.EA),
+      DST_ENC.Supplement
+    );
+}
 }
 
 foreach cc = [ "cc", "ls", "lt", "eq", "mi", "f", "ne", "ge",
                "cs", "pl", "gt", "t", "hi", "vc", "le", "vs"] in {
 def SET#"d8"#cc : MxSccR<cc>;
-def SET#"j8"#cc : MxSccM<cc, MxType8.JOp, MxType8.JPat, MxEncEAj_0, MxExtEmpty>;
-def SET#"p8"#cc : MxSccM<cc, MxType8.POp, MxType8.PPat, MxEncEAp_0, MxExtI16_0>;
+def SET#"j8"#cc : MxSccM<cc, MxType8.JOp, MxType8.JPat, MxEncAddrMode_j<"dst">>;
+def SET#"p8"#cc : MxSccM<cc, MxType8.POp, MxType8.PPat, MxEncAddrMode_p<"dst">>;
 }
 
 //===----------------------------------------------------------------------===//
 // Jumps
 //===----------------------------------------------------------------------===//
 
 ///------------------------------+---------+---------
 /// F  E  D  C  B  A  9  8  7  6 | 5  4  3 | 2  1  0

 /// --------------------------------------------------
 ///  0  1  1  0 | CONDITION |   8-BIT DISPLACEMENT
 /// --------------------------------------------------
 ///  16-BIT DISPLACEMENT IF 8-BIT DISPLACEMENT = $00
 /// --------------------------------------------------
 ///  32-BIT DISPLACEMENT IF 8-BIT DISPLACEMENT = $FF
 /// --------------------------------------------------
 let isBranch = 1, isTerminator = 1, Uses = [CCR] in
-class MxBcc<string cc, Operand TARGET, MxEncoding ENC = MxEncEmpty>
-    : MxInst<(outs), (ins TARGET:$dst), "b"#cc#"\t$dst", [], ENC>;
+class MxBcc<string cc, Operand TARGET, dag disp, dag supplement>
+    : MxInst<(outs), (ins TARGET:$dst), "b"#cc#"\t$dst", []> {
+  let Inst =
+      (ascend
+        (descend 0b0110, !cast<MxEncCondOp>("MxCC"#cc).Value, disp),
+        supplement
+      );
+}
 
 foreach cc = [ "cc", "ls", "lt", "eq", "mi", "ne", "ge",
                "cs", "pl", "gt", "hi", "vc", "le", "vs"] in {
   def B#cc#"8"
     : MxBcc<cc, MxBrTarget8,
-            MxEncoding<MxBead8Disp<0>,
-                       !cast<MxBead4Bits>("MxCC"#cc), MxBead4Bits<0x6>>>;
+        (operand "$dst", 8, (encoder "encodePCRelImm<8>")), (ascend)>;
+
   def B#cc#"16"
-    : MxBcc<cc, MxBrTarget16,
-            MxEncoding<MxBead4Bits<0x0>,
-                       MxBead4Bits<0x0>, !cast<MxBead4Bits>("MxCC"#cc),
-                       MxBead4Bits<0x6>, MxBead16Imm<0>>>;
+    : MxBcc<cc, MxBrTarget16, (descend 0b00000000),
+        (operand "$dst", 16, (encoder "encodePCRelImm<16>"))>;
 }
 
 foreach cc = [ "cc", "ls", "lt", "eq", "mi", "ne", "ge",
                "cs", "pl", "gt", "hi", "vc", "le", "vs"] in {
 def : Pat<(MxBrCond bb:$target, !cast<PatLeaf>("MxCOND"#cc), CCR),
           (!cast<Instruction>("B"#cc#"8") MxBrTarget8:$target)>;
 }
 
 /// -------------------------------------------------
 ///  F  E  D  C  B  A  9  8 | 7  6  5  4  3  2  1  0
 /// -------------------------------------------------
 ///  0  1  1  0  0  0  0  0 |   8-BIT DISPLACEMENT
 /// -------------------------------------------------
 ///  16-BIT DISPLACEMENT IF 8-BIT DISPLACEMENT = $00
 /// -------------------------------------------------
 ///  32-BIT DISPLACEMENT IF 8-BIT DISPLACEMENT = $FF
 /// -------------------------------------------------
 let isBranch = 1, isTerminator = 1, isBarrier=1 in
 class MxBra<Operand TARGET, MxEncoding ENC = MxEncEmpty>
     : MxInst<(outs), (ins TARGET:$dst), "bra\t$dst", [], ENC>;
 
 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>>>;
 
 def : Pat<(br bb:$target), (BRA8 MxBrTarget8:$target)>;
 
 
 //===----------------------------------------------------------------------===//
 // Call
 //===----------------------------------------------------------------------===//
 
 // All calls clobber the non-callee saved registers. %SP is marked as
 // a use to prevent stack-pointer assignments that appear immediately
 // before calls from potentially appearing dead. Uses for argument