This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [06/11] TI C6X binutils port: include/


+/* Define the CTRL macro before including this file; it takes a name
+   and the fields from tic6x_ctrl.  */

You should describe what the five parameters mean, too, or reference the
location of the descriptions if they're described elsewhere.

+CTRL(amr, C62X, read_write, 0x0, 0x10),

Perhaps the trailing comma should be part of the CTRL macro, not
explicit here, in case some future use needs punctuation other than a
comma?

+/* Define the FMT macro before including this file; it takes a name
+   and the fields from tic6x_insn_format.  */
+FMT(...),

Likewise, and for the other files.

Any particular reason why all those opcode tables went in include/ and
not opcodes/ ?  Is it just for the index enums?

+  } tic6x_insn_field_id;
+typedef struct

Please put blank lines between large definitions, for readability.

+     8).  Ignored for constant operands.  */
+  unsigned int size;
+  /* Whether the operand is read, written or both.  In addition to the

Hmmm.. I wonder if a blank line is warranted in cases like these, for
readability...  not needed if you have syntax highlighting, but without
it?

Whyu does include/elf/tic6x.h say it's part of BFD, but none of the
others do?

Otherwise, this looks OK.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]