[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/XtensaInstrFormats.td:31
+                   InstrItinClass itin = NoItinerary>:
+                   XtensaInst<3, outs, ins, asmstr, pattern, itin>
+{
----------------
andreisfr wrote:
> craig.topper wrote:
> > 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;                                                                                                                                                                                                                                                               
> > }       
> > ```
> @craig.topper , I corrected instruction formats and instruction descriptions, do you think that this code now in common style or it must be corrected bit more?
I think this looks better. Unfortunately, we don't have a formatting tool for tablegen and no rules are written down. I think this is good enough.


================
Comment at: llvm/lib/Target/Xtensa/XtensaInstrFormats.td:166
+
+
+  let Inst{23-12} = imm12;
----------------
Drop the second blank line?


================
Comment at: llvm/lib/Target/Xtensa/XtensaRegisterInfo.td:30
+class ARReg<bits<4> num, string n, list<string> alt = []> : XtensaReg<n> {
+    let HWEncoding{3-0} = num;
+    let AltNames = alt;
----------------
This is overindented relative to the style of the rest of the file.


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

https://reviews.llvm.org/D64830