[PATCH] D64830: [Xtensa 4/10] Add basic *td files with Xtensa architecture description.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/Xtensa/Xtensa.td:1
+//===- Xtensa.td - Describe the Xtensa Target Machine -----------*- tablegen -*-===//
+//
----------------
Please limit line length to 80 characters


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrFormats.td:1
+//===- XtensaInstrFormats.td - Xtensa Instruction Formats -------*- tablegen -*-===//
+//
----------------
Please limit line length to 80 characters


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrFormats.td:14
+                 InstrItinClass itin = NoItinerary>: Instruction
+{
+  let Namespace = "Xtensa";
----------------
I think the prevailing style in LLVM has the open curly brace at the end of the previous line.


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrFormats.td:31
+                   InstrItinClass itin = NoItinerary>:
+                   XtensaInst<3, outs, ins, asmstr, pattern, itin>
+{
----------------
It might just be me, but I find this lining the XtensaInst up with template parameters hard to read.  I think a common style is to put at the start of the line with only a small indentation. For example

```
class PseudoI<dag oops, dag iops, list<dag> pattern>                                                                                                                                                                                                                                   
  : X86Inst<0, Pseudo, NoImm, oops, iops, ""> {                                                                                                                                                                                                                                        
  let Pattern = pattern;                                                                                                                                                                                                                                                               
}       
```


================
Comment at: llvm/lib/Target/Xtensa/XtensaOperands.td:1
+//===- XtensaOperands.td - Xtensa instruction operands ----*- tblgen-*--===//
+//
----------------
This line seems shorter than 80 characters.


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

https://reviews.llvm.org/D64830