RE: [Patch, microblaze, bfd/gas/include] Add MicroBlaze TLS Support

[David Holsgrove <david dot holsgrove at xilinx dot 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?
> 
> +  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().
> 
> +          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.
> 
> +                      check_unique_offset( off, tls_type, 0xDDDDDDDD);
> 
> What is 0xDDDDDDDD?
> 
> +      size *= (sizeof (*local_got_refcounts)
> +               + sizeof (*local_got_tls_masks));
> 
> One line.
> 

I've attached v2 of the patch removing debug code, fixing indentation and parenthesis.

thanks again,
David

> 
> --
> Michael Eager	 eager@eagercon.com
> 1960 Park Blvd., Palo Alto, CA 94306  650-325-8077
> 
>

Attachment: 0002-Add-microblaze-TLS-support.patch
Description: 0002-Add-microblaze-TLS-support.patch