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