This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
RE: [Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support
- From: David Holsgrove <david dot holsgrove at xilinx dot com>
- To: Michael Eager <eager at eagerm dot com>
- Cc: "binutils at sourceware dot org" <binutils at sourceware dot org>, John Williams <jwilliams at xilinx dot com>, Vinod Kathail <vinodk at xilinx dot com>, Tom Shui <tshui at xilinx dot com>, Vidhumouli Hunsigida <vidhum at xilinx dot com>, Nagaraju Mekala <nmekala at xilinx dot com>, "Edgar E. Iglesias" <edgar dot iglesias at gmail dot com>
- Date: Thu, 29 Nov 2012 07:26:34 +0000
- Subject: RE: [Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support
- References: <0fb136ec-f235-43dc-ace2-74869031d8f0@AM1EHSMHS012.ehs.local> <50B6D220.5050604@eagerm.com>
Hi Michael,
> -----Original Message-----
> From: Michael Eager [mailto:eager@eagerm.com]
> Sent: Thursday, 29 November 2012 1:10 pm
> To: David Holsgrove
> Cc: binutils@sourceware.org; John Williams; Vinod Kathail; Tom Shui; Vidhumouli
> Hunsigida; Nagaraju Mekala; Edgar E. Iglesias
> Subject: Re: [Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support
>
> On 11/22/2012 04:06 AM, David Holsgrove wrote:
> >
> > Add support for handling TLS symbol suffixes and generating
> > TLS relocs for General Dynamic and Local Dynamic models.
> >
> > Check in generated files that have changed as a result of new
> > TLS Relocs
>
> +check_unique_offset(bfd_vma offset, int tls_type, bfd_vma v)
> +{
> + static bfd_vma offsets[1000];
> + static bfd_vma values[1000];
>
> Why 1000?
>
This is inherited code from out of tree sources, and it would appear that this function
check_unique_offset is dead debug code, which I missed as it simply returns every
time it is called without doing anything;
static void
check_unique_offset(bfd_vma offset, int tls_type, bfd_vma v)
{
static bfd_vma offsets[1000];
static bfd_vma values[1000];
static int noffsets = 0;
int scan1;
return;
> + for (scan1 = 0; scan1 < noffsets; scan1++)
> + {
> + if (offsets[scan1] == offset)
> + {
> + fprintf (stderr, "Duplicate Offset: %lx type: %x Old: %lx New: %lx
> \n",
> + offset, tls_type, values[scan1], v);
> + }
> + }
> + fprintf (stderr, "%d Registered Offset: %lx Value: %lx type: %x\n",
> + noffsets, offset, v, tls_type);
>
> Please fix indent. There are several places where the indent is incorrect.
>
> Is this debugging code? Are the messages errors which you expect to
> generate? If yes, use bfd_perror().
>
So I'll remove this function as unnecessary.
> + if ( IS_TLS_LD (tls_type) )
>
> No spaces after/before parens surrounding expression. There are other places
> which have extra spaces.
>
> + abort();
>
> Space before paren in function calll. There are other places which need spaces
> before parens.
Thanks, will fix in the next version.
>
> + check_unique_offset( off, tls_type, 0xDDDDDDDD);
>
> What is 0xDDDDDDDD?
>
All calls to check_unique_offset can be removed.
> + size *= (sizeof (*local_got_refcounts)
> + + sizeof (*local_got_tls_masks));
>
> One line.
>
Thanks again, will forward v2 of patch shortly.
David
>
> --
> Michael Eager eager@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
>
>