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

Joseph S. Myers joseph@codesourcery.com
Wed Mar 24 00:30:00 GMT 2010


On Tue, 23 Mar 2010, DJ Delorie wrote:

> 
> +/* 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.

The meaning is the fields of the tic6x_ctrl structure.  I've made these 
comments reference tic6x.h for the structure definitions.  I've also made 
them more precise about how the macro arguments get modified by 
concatenation, and corrected cases where the comments said the arguments 
were a name *and* structure fields when the name is in fact the first 
structure field.

> +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?

I've made that change.  (There's an obvious corresponding change to macro 
definitions in the opcodes/ patch; only the new include/ patch is 
attached.)

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

Yes.  The individual values of the index enum for the opcode table may not 
yet be used, but they will be once the resource constraint checks are 
implemented (there will be a table in gas listing pairs of instructions 
that cannot go in the same execute packet).

> 
> +  } 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?

I've added blank lines between definitions and within structure 
definitions.

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

Files in include/elf generally mention BFD (and are used there).  Files in 
include/opcode generally don't - after all, at least for C6X BFD doesn't 
use any of them - but mention a random mixture of GAS, GDB and binutils 
described in various ways; following those files that don't try to mention 
particular programs seemed simplest there.

-- 
Joseph S. Myers
joseph@codesourcery.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: c6x-06a-include.bz2
Type: application/octet-stream
Size: 15225 bytes
Desc: 
URL: <https://sourceware.org/pipermail/binutils/attachments/20100324/775812b8/attachment.obj>


More information about the Binutils mailing list