[M68k] Use fixed asm string for MxPseudo instructions

[ricky26]

Summary

This is required because empty strings are not allowed when generating
the assembly parser tables.

[RKSimon]

@myhsu Would we be better off adding non-empty asm strings to all uses?

[myhsu]

In D98532#2626683, @RKSimon wrote:

@myhsu Would we be better off adding non-empty asm strings to all uses?

Do you mean setting non-empty asm string by default on MxInst? I feel like the non-empty string requirement imposed by parser table can help us catch mistakes if we forget to set assembly string for an instruction (in the future), so I'm incline not to do that

[RKSimon]

In D98532#2627487, @myhsu wrote:

In D98532#2626683, @RKSimon wrote:

@myhsu Would we be better off adding non-empty asm strings to all uses?

Do you mean setting non-empty asm string by default on MxInst? I feel like the non-empty string requirement imposed by parser table can help us catch mistakes if we forget to set assembly string for an instruction (in the future), so I'm incline not to do that

I meant just stop MxPseudo taking a default string so all instances include a useful asm comment string.

[myhsu]

In D98532#2629887, @RKSimon wrote:

In D98532#2627487, @myhsu wrote:

In D98532#2626683, @RKSimon wrote:

@myhsu Would we be better off adding non-empty asm strings to all uses?

Do you mean setting non-empty asm string by default on MxInst? I feel like the non-empty string requirement imposed by parser table can help us catch mistakes if we forget to set assembly string for an instruction (in the future), so I'm incline not to do that

I meant just stop MxPseudo taking a default string so all instances include a useful asm comment string.

Ah, using the asm string as comments (for MxPseudo instances), I see. But I don't think these pseudo instructions will ever be printed/parsed to/from assembly (so it can't be used as comments in assembly code). From what I understand they're only syntax sugar for codegen (unlike some of the targets that have assembler pseudo instructions). If that's the case, I don't think it's really necessary to put meaningful string in the assembly string. If we want to add comments (for documentation purpose rather than real comments in the assembly code), I prefer to put in the TG code.

[RKSimon]

OK fair enough, no more comments.

[myhsu]

LGTM

llvm/lib/Target/M68k/M68kInstrCompiler.td

 //
 // CMOV* - Used to implement the SELECT DAG operation. Expanded after
 // instruction selection into a branch sequence.
 //===----------------------------------------------------------------------===//
 
 let usesCustomInserter = 1, Uses = [CCR] in
 class MxCMove<MxType TYPE>
     : MxPseudo<(outs TYPE.ROp:$dst), (ins TYPE.ROp:$t, TYPE.ROp:$f, i8imm:$cond),
-               "",
                [(set TYPE.VT:$dst,
                      (TYPE.VT (MxCmov TYPE.VT:$t, TYPE.VT:$f, imm:$cond, CCR)))]>;
 
 def CMOV8d  : MxCMove<MxType8d>;
 def CMOV16d : MxCMove<MxType16d>;
 def CMOV32r : MxCMove<MxType32r>;
 
 
 //===----------------------------------------------------------------------===//
 // Calls
 //===----------------------------------------------------------------------===//
 
 // ADJCALLSTACKDOWN/UP implicitly use/def %SP because they may be expanded into
 // a stack adjustment and the codegen must know that they may modify the stack
 // pointer before prolog-epilog rewriting occurs.
 // Pessimistically assume ADJCALLSTACKDOWN / ADJCALLSTACKUP will become
 // sub / add which can clobber CCR.
 let Defs = [SP, CCR], Uses = [SP] in {
 
   def ADJCALLSTACKDOWN
-    : MxPseudo<(outs), (ins i32imm:$amt1, i32imm:$amt2), "#ADJCALLSTACKDOWN",
+    : MxPseudo<(outs), (ins i32imm:$amt1, i32imm:$amt2),
                [(MxCallSeqStart timm:$amt1, timm:$amt2)]>;
 
   def ADJCALLSTACKUP
-    : MxPseudo<(outs), (ins i32imm:$amt1, i32imm:$amt2), "#ADJCALLSTACKUP",
+    : MxPseudo<(outs), (ins i32imm:$amt1, i32imm:$amt2),
                [(MxCallSeqEnd timm:$amt1, timm:$amt2)]>;
 
 } // Defs
 
 //===----------------------------------------------------------------------===//
 // Tail Call
 //===----------------------------------------------------------------------===//
 

 // check if the current stacklet has enough free memory. If it does, memory is
 // allocated by bumping the stack pointer. Otherwise memory is allocated from
 // the heap.
 //===----------------------------------------------------------------------===//
 
 let Defs = [SP, CCR], Uses = [SP] in
 let usesCustomInserter = 1 in
 def SALLOCA : MxPseudo<(outs MxARD32:$dst), (ins MxARD32:$size),
-                       "# variable sized alloca for segmented stacks",
                        [(set iPTR:$dst, (MxSegAlloca iPTR:$size))]>;

llvm/lib/Target/M68k/M68kInstrControl.td

 
 // TODO Implement LINK/UNLK
 
 let isTerminator = 1, isReturn = 1, isBarrier = 1, hasCtrlDep = 1 in {
 
 def RTS : MxInst<(outs), (ins), "rts", [], MxEncFixed<0x4E75>>;
 
 let isCodeGenOnly = 1 in
-def RET : MxPseudo<(outs), (ins i32imm:$adj, variable_ops), "",
+def RET : MxPseudo<(outs), (ins i32imm:$adj, variable_ops),
                    [(MxRet timm:$adj)]>;
 } // isTerminator = 1, isReturn = 1, isBarrier = 1, hasCtrlDep = 1
 
 //===----------------------------------------------------------------------===//
 // SETCC_C Patterns
 //===----------------------------------------------------------------------===//
 
 // Use subx to materialize carry bit.
 let Uses = [CCR], Defs = [CCR], isPseudo = 1 in {
 // FIXME These are pseudo ops that should be replaced with Pat<> patterns.
 // However, Pat<> can't replicate the destination reg into the inputs of the
 // result.
-def SETCS_C8d : MxPseudo<(outs MxDRD8:$dst), (ins), "",
+def SETCS_C8d : MxPseudo<(outs MxDRD8:$dst), (ins),
                          [(set MxDRD8:$dst, (MxSetCC_C MxCONDcs, CCR))]>;
-def SETCS_C16d : MxPseudo<(outs MxDRD16:$dst), (ins), "",
+def SETCS_C16d : MxPseudo<(outs MxDRD16:$dst), (ins),
                           [(set MxDRD16:$dst, (MxSetCC_C MxCONDcs, CCR))]>;
-def SETCS_C32d : MxPseudo<(outs MxXRD32:$dst), (ins), "",
+def SETCS_C32d : MxPseudo<(outs MxXRD32:$dst), (ins),
                           [(set MxXRD32:$dst, (MxSetCC_C MxCONDcs, CCR))]>;
 } // Uses = [CCR], Defs = [CCR], isPseudo = 1
 
 
 def : Pat<(i16 (anyext (i8 (MxSetCC_C MxCONDcs, CCR)))), (SETCS_C16d)>;
 def : Pat<(i32 (anyext (i8 (MxSetCC_C MxCONDcs, CCR)))), (SETCS_C32d)>;
 
 def : Pat<(i16 (sext (i8 (MxSetCC_C MxCONDcs, CCR)))), (SETCS_C16d)>;

llvm/lib/Target/M68k/M68kInstrData.td

 def POP32r : MxPseudo<(outs XR32:$reg), (ins)>;
 } // let mayLoad = 1
 
 } // let Uses/Defs = [SP], hasSideEffects = 0
 
 
 let Defs = [CCR] in {
 class MxPseudoMove_RR<MxType DST, MxType SRC, list<dag> PAT = []>
-    : MxPseudo<(outs DST.ROp:$dst), (ins SRC.ROp:$src), "", PAT>;
+    : MxPseudo<(outs DST.ROp:$dst), (ins SRC.ROp:$src), PAT>;
 
 class MxPseudoMove_RM<MxType DST, MxOperand SRCOpd, list<dag> PAT = []>
-    : MxPseudo<(outs DST.ROp:$dst), (ins SRCOpd:$src), "", PAT>;
+    : MxPseudo<(outs DST.ROp:$dst), (ins SRCOpd:$src), PAT>;
 }
 
 /// This group of Pseudos is analogues to the real x86 extending moves, but
 /// since M68k does not have those we need to emulate. These instructions
 /// will be expanded right after RA completed because we need to know precisely
 /// what registers are allocated for the operands and if they overlap we just
 /// extend the value if the registers are completely different we need to move
 /// first.

llvm/lib/Target/M68k/M68kInstrFormats.td

class MxInst<dag outs, dag ins,
 
   // Number of bytes
   let Size = 0;
 
   let UseLogicalOperandMappings = 1;
 }
 
 // M68k PSEUDO INSTRUCTION
-class MxPseudo<dag outs, dag ins, string asmStr = "", list<dag> pattern = []>
-    : MxInst<outs, ins, asmStr, pattern> {
+class MxPseudo<dag outs, dag ins, list<dag> pattern = []>
+    : MxInst<outs, ins, "; error: this should not be emitted", pattern> {
   let isPseudo = 1;
 }