[Fwd: Re: [PATCH] [ARC] Add linker relaxation.]

[Claudiu Zissulescu]

Forgot to forward to the group.
//Claudiu

-------- Forwarded Message --------
From: Claudiu Zissulescu <claziss@synopsys.com>
To: Nick Clifton <nickc@redhat.com>
Subject: Re: [PATCH] [ARC] Add linker relaxation.
Date: Wed, 24 Jul 2019 17:26:52 +0300

> Hi Nick,
> 
> On Wed, 2019-07-24 at 15:07 +0100, Nick Clifton wrote:
> > Hi Claudiu,
> > 
> > > Add linker relaxation. The first relaxation added is converting
> > > GOTPC32 to PCREL relocations. This relaxation doesn't change the size of
> > > the binary.
> > 
> > As a matter of interest, what is the benefit of this relaxation ?
> > If it does not change the size then why bother ?
> 
> Speed :) we replace a load instruction with an add.
> 
> > > bfd/
> > > xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>
> > > 
> > > 	* elf32-arc.c (bfd_get_32_me): New function.
> > > 	(bfd_put_32_me): Likewise.
> > > 	(arc_elf_relax_section): Likewise.
> > > 	(bfd_elf32_bfd_relax_section): Define.
> > > 
> > > ld/testsuite/
> > > xxxx-xx-xx  Claudiu Zissulescu  <claziss@synopsys.com>
> > > 
> > > 	* ld-arc/relax-local-pic.d: New test.
> > > 	* ld-arc/relax-local-pic.s: New file.
> > 
> > Given that these changes are ARC specific, you can self-approve.
> > Or were you just posting here to let us know what you are doing ?
> > (If so, then it helps to include "COMMIT" in the subject line,
> > so that I know that the patch does not need a review).
> 
> I've already pushed it :) Next time I will add COMMIT in the subject. Please
> apologize me for this.
> 
> > > +static bfd_vma
> > > +bfd_get_32_me (bfd * abfd,const unsigned char * data)
> > > +{
> > > +  bfd_vma value = 0;
> > > +
> > > +  if (bfd_big_endian(abfd)) {
> > > +    value = bfd_get_32 (abfd, data);
> > > +  }
> > > +  else {
> > > +    value = ((bfd_get_8 (abfd, data) & 255) << 16);
> > > +    value |= ((bfd_get_8 (abfd, data + 1) & 255) << 24);
> > > +    value |= (bfd_get_8 (abfd, data + 2) & 255);
> > > +    value |= ((bfd_get_8 (abfd, data + 3) & 255) << 8);
> > > +  }
> > 
> > Another curiosity - why "_me" ?  Does it stand for middle-endian ?
> 
> It is the "middle-endian" thing specific to ARC processors.
> 
> > Also - formatting.  Please do not end lines with opening curly braces.
> 
> Aghh, it escaped me this, I will push an cleanup asap.
> 
> > But apart from that the patch looks good to me.
> > 
> 
> Thanks!