[PATCH] D144154: [X86]Use Class to refactor ArithMetic td file in X86

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:467
+class UnaryOpM<bits<8> opcode, Format f, string mnemonic, X86TypeInfo info,
+              list<dag> pattern>
+  : ITy<opcode, f, info, (outs), (ins info.MemOperand:$dst), mnemonic,
----------------
Identation is off here


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:473
+class UnaryOpR_mul<bits<8> opcode, Format f, string mnemonic, X86TypeInfo info,
+              list<dag> pattern>
+  : ITy<opcode, f, info, (outs), (ins info.RegClass:$src), mnemonic,
----------------
and here


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:486
+class UnaryOpR_C<bits<8> opcode, Format f, string mnemonic, X86TypeInfo info,
+                  list<dag> pattern>
+  : ITy<opcode, f, info, (outs info.RegClass:$dst),
----------------
and here


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:492
+class INCDECR<Format f, string mnemonic, X86TypeInfo info,
+                SDPatternOperator node>
+  : UnaryOpR_C<0xFE, f, mnemonic, info,
----------------
and here.


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:506
+  : UnaryOpR_C<opcode, AddRegFrm, mnemonic, info, []>, 
+                Requires<[Not64BitMode]>{
+  let Opcode = opcode;
----------------
This Requires looks kind of funny. Might be better to switch to `let Predicates = [Not64BitMode]`


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:560
+class BinOpRRxMI<bits<8> opcode, Format f, string mnemonic, X86TypeInfo info, 
+                dag inlist, list<dag> pattern>
+  : ITy<opcode, f, info, (outs info.RegClass:$dst), inlist, mnemonic, 
----------------
indentation is off


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:569
+                (X86smul_flag info.RegClass:$src1, info.RegClass:$src2))]>,
+                Sched<[sched]>, TB;
+
----------------
This is probably indented too far. It looks like its part of the `[` expression


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:573
+class IMulOpRM<bits<8> opcode, string mnemonic, X86TypeInfo info, 
+                X86FoldableSchedWrite sched>
+  : BinOpRM_C<opcode, MRMSrcMem, mnemonic, info,
----------------
indentation


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:581
+class IMulOpRRI8<bits<8> opcode, string mnemonic, X86TypeInfo info, 
+                  X86FoldableSchedWrite sched>
+  : BinOpRRxMI<opcode, MRMSrcReg, mnemonic, info, 
----------------
indentation


================
Comment at: llvm/lib/Target/X86/X86InstrArithmetic.td:583
+  : BinOpRRxMI<opcode, MRMSrcReg, mnemonic, info, 
+              (ins info.RegClass:$src1, 
+              info.Imm8Operand:$src2), 
----------------
indentation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144154