[PATCH] D64830: [Xtensa 4/10] Add basic *td files with Xtensa architecture description.
Craig Topper via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 1 18:53:50 PDT 2021
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
More information about the llvm-commits
mailing list