public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Bernd Schmidt <bernds@codesourcery.com>
To: "Joseph S. Myers" <joseph@codesourcery.com>
Cc: binutils@sources.redhat.com, Paul Brook <paul@codesourcery.com>
Subject: Re: Patch: Add c6x-uclinux support
Date: Mon, 28 Mar 2011 23:18:00 -0000	[thread overview]
Message-ID: <4D9116AD.9040809@codesourcery.com> (raw)
In-Reply-To: <Pine.LNX.4.64.1103282214490.30472@digraph.polyomino.org.uk>

On 03/29/2011 12:33 AM, Joseph S. Myers wrote:
> On Fri, 11 Mar 2011, Bernd Schmidt wrote:
> 
>> Also included in this patch is support for a new assembler directive,
>> ".scomm", used to better support small-data common symbols as required
>> by the ABI. There's also a small assembler bugfix for -mgenerate-rel.
> 
> Why exactly is a .scommon section needed in addition to .bss and .far - 
> how does it differ from those two sections?

What happens on other targets is that the linker decides, based on size,
whether a common symbol goes into near or far BSS. That doesn't work for
the C6X ABI where considerations such as symbol type come into play.
Hence, the compiler must make the decision.

> Also, this directive appears to generate SHN_TIC6X_SCOMMON in object 
> files.  I don't see any sign of this in the ABI ("There are no 
> processor-specific special section indexes defined.") - exactly what 
> specification of it has been agreed with TI?

I described the mechanism, including the fact that this value should be
reserved in the ABI, to Alan Davis and he replied with "OK, good." (Nov 17).

> I also notice that the diagnostics in .scomm parsing often fail
> to follow the GNU Coding Standards (other diagnostics in tc-tic6x.c make 
> sure to avoid initial capital letters or trailing '.'; they also avoid the 
> grave accent as an opening quote, although this last style point, though 
> consistent with GCC, is outside the GNU Coding Standards).

That's probably just copied from the m32r implementation. Will fix.

> * I presume semantics for R_C6000_EHTYPE, R_C6000_PCR_H16, and 
> R_C6000_PCR_L16 are to be added in subsequent patches, since only the 
> definitions and nothing else are added in this patch.

Yes. The EH bits I've left for Paul to submit, no code exists yet for
the others as they were defined only recently.

> * I think the static functions in elf32-tic6x.c should be consistently 
> named with an elf32_tic6x_ prefix (e.g. using_dsbt, install_rela, 
> make_got_dynreloc).

Can do, but any particular reason? To be honest I'd kind of prefer to
remove it from static function since it doesn't add any non-obvious
information.

> * What is the purpose of the ELF_MAXPAGESIZE change (and the logic behind 
> the choice of the new value)?  I don't think the value is of much 
> significance, but it would be good to know why the change was needed.

Can't quite remember to be honest. I've just tried with it set to 1, and
the linker behaves oddly - it to generate rather more LOAD headers, and
ld.so complains that it can't map the library if libc.so is compiled
with such a linker.

> * There is a reference to a symbol "__DYNAMIC", which I think should be 
> "_DYNAMIC".  (bfin has a reference to "__DYNAMIC", but I think that's 
> because it deviates from the gABI in having a leading underscore on symbol 
> names, unlike C6X.)

Will fix.

> * Why does elf32_tic6x_gc_sweep_hook handle R_C6000_SBR_GOT_U15_W 
> differently from R_C6000_SBR_GOT_L16_W and R_C6000_SBR_GOT_H16_W?  I 
> haven't tried to construct a testcase showing this is a problem, but it 
> seems strange for it to do so without a comment.

Seems to be an oversight.

> * There is a test for the undefined weak symbol handling for 
> R_C6000_PCR_S21, in the case where the instruction has the specified form 
> to be converted to a branch to the return address; tests for the other 
> cases with required semantics (absolute and SB-relative relocations 
> resolving to 0 and to the static base respectively) would be good.

The former exists in the shared library/executable testcases.


Bernd

  reply	other threads:[~2011-03-28 23:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-11 11:42 Bernd Schmidt
2011-03-28 22:34 ` Joseph S. Myers
2011-03-28 23:18   ` Bernd Schmidt [this message]
2011-03-29 16:13     ` Joseph S. Myers
2011-03-29 22:59       ` Bernd Schmidt
2011-03-29 23:42         ` Joseph S. Myers
2011-03-30  8:23           ` Bernd Schmidt
2011-03-30 10:14             ` Joseph S. Myers
2011-03-30 11:12               ` Bernd Schmidt
2011-03-30 11:19                 ` Joseph S. Myers
2011-03-30 18:46                   ` Bernd Schmidt
2011-03-30 19:34                     ` Joseph S. Myers
2011-03-31 14:40                     ` Tristan Gingold
2011-03-31 16:14                       ` Bernd Schmidt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D9116AD.9040809@codesourcery.com \
    --to=bernds@codesourcery.com \
    --cc=binutils@sources.redhat.com \
    --cc=joseph@codesourcery.com \
    --cc=paul@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).