PATCH: support for NEC SX architecture

Tristan Gingold gingold@adacore.com
Wed Jun 3 13:09:00 GMT 2009


>> You'd better to submit your patches inline (not as a tar.gz) as it is
>> easier to review.
> well, the patches are quite big, so I though it would be nicer to pack
> them - didnt really expect someone would review them in their email
> client ... I can resubmit them inline, if necessary.

One usual way is to split the patch (as you did) and post part by part.

>> It is strange to use bfd_vma type for f_nsyms in include/coff/ 
>> internal.h
> true. however, I needed a type with the same domain as the virtual
> addresses of the target CPU: any suggestions?

Requiring the same domain is too strong here.  You can have 32bits  
targets even when
bfd_vma is 64bits.  If you need a domain larger or equal than virtual  
addresses domain,
bfd_vma will fit this purpose although you may prefer bfd_size_type.

>> Why are you using SX_PACKED in declaration of external_filhdr,
>> external_aouthdr ... ?
> just to make sure that the compiler does not introduce any padding as
> these structures should correspond to the memory/file contents of an  
> SX
> COFF file, bit by bit. it's an sx target specific define that  
> translates
> to gcc packed attribute. I tend to abstract gccisms away with defines
> whenever possible.

All the structures (but external_syment) have only char[] fields.   
external_syment has a char * field
but within a union with at least a larger field.  So padding won't be  
inserted and I think this is not
necessary.
Using a gcc specific attribute may break other compilers (but I agree  
this is not critical).

>> Although I am ready to read your patch (when inlined), I can't  
>> approve
>> it.
> I thought so, it was more a question for Nick or anybody else who can
> say yes to some of the changes in the core coff code - it's those that
> probably need review most, not the target specific code.

Fine!

>> BTW: in there a documentation for the cpu ?
> sure there is. ;) it is unfortunately not (or, better, is not supposed
> to be) publicly available. however, asking google about it, you just
> might find something. ;)

Thanks,

Tristan.



More information about the Binutils mailing list