Re: New assembler warnings with binutils 2.29

2017-08-17 Thread Laura Abbott
On 08/14/2017 07:45 AM, Ramana Radhakrishnan wrote:
> On Mon, Aug 14, 2017 at 11:34:45AM +0100, Catalin Marinas wrote:
>>>
>>> Any attempts to realias these to the correct meaning as per the AAPCS
>>> continues to work from my limited testing of a prototype patch so far.
>>
>> This would be great. Thanks Ramana!
> 
> I've submitted the patch upstream and would appreciate any testing of kernel
> builds to double check that this fixes the issue at hand.
> 
> https://sourceware.org/ml/binutils/2017-08/msg00160.html

I verified on a local build that this makes the warnings go away.

Thanks,
Laura

> 
> 
> Ramana


Re: New assembler warnings with binutils 2.29

2017-08-14 Thread Ramana Radhakrishnan
On Mon, Aug 14, 2017 at 11:34:45AM +0100, Catalin Marinas wrote:
> > 
> > Any attempts to realias these to the correct meaning as per the AAPCS
> > continues to work from my limited testing of a prototype patch so far.
> 
> This would be great. Thanks Ramana!

I've submitted the patch upstream and would appreciate any testing of kernel
builds to double check that this fixes the issue at hand.

https://sourceware.org/ml/binutils/2017-08/msg00160.html


Ramana

> 
> -- 
> Catalin


Re: New assembler warnings with binutils 2.29

2017-08-14 Thread Catalin Marinas
On Mon, Aug 14, 2017 at 10:55:48AM +0100, Ramana Radhakrishnan wrote:
> On Mon, Aug 14, 2017 at 09:59:02AM +0100, Catalin Marinas wrote:
> > On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> > > On 11 August 2017 at 10:22, Catalin Marinas  
> > > wrote:
> > > > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> > > >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> > > >> to produce new warnings:
> > > >>
> > > >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> > > >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
> > > >> redefine built-in register 'lr'
> > > >>
> > > >> This is
> > > >>
> > > >> /*
> > > >>  * Register aliases.
> > > >>  */
> > > >> lr  .reqx30 // link register
> > > >
> > > > Strange, does gas now think 'lr' is a general purpose register (aliased
> > > > to x30)? It never was and IIRC the toolchain people many years ago
> > > > refused to add it, hence the alias above in the kernel. I wonder if they
> > > > added 'fp' as well...
> 
> I don't remember the reasons why it was refused in the past. Asking around I
> can't seem to find anyone who remembers why either and it would be much
> quicker to fix the problem in binutils than any of this archeology :)

I don't have the old emails around either ;).

> > > > We could remove the alias and replace all 'lr' instances with 'x30'
> > > > throughout the kernel (no too many) or we add some #ifdef around the
> > > > above based on the binutils version.
> > > > This is annoying. Replacing lr with x30 achieves the opposite of the
> > > intent of the binutils change. And using #ifdefs is inaccurate,
> > > because you can't really test the binutils version only the GCC
> > > version, and those are not tightly coupled.
> > > > Can you .unreq it?
> > 
> > Not really, with an older binutils I get:
> > 
> > arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'
> > 
> > I personally consider this a binutils bug. After 6+ years (probably not
> > all public) of building the kernel just fine, all of a sudden certain
> > strings became reserved in gas. Three options:
> 
> IIRC the ABI states that LR is an alias for X30, FP is an alias for X29,
> IP0 an alias for X16 and IP1 an alias for X17.

It's unfortunate that binutils never define them, so we ended up with
.req in the kernel.

> The warning is unfortunate - looking at the implementation of the
> .req directive in gas it's something controlled in the aarch64 backend
> and thus we could change gas to make LR (and friends) an internal alias thus
> removing the warning.
> 
> Any attempts to realias these to the correct meaning as per the AAPCS
> continues to work from my limited testing of a prototype patch so far.

This would be great. Thanks Ramana!

-- 
Catalin


Re: New assembler warnings with binutils 2.29

2017-08-14 Thread Ramana Radhakrishnan

On Mon, Aug 14, 2017 at 09:59:02AM +0100, Catalin Marinas wrote:

On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> On 11 August 2017 at 10:22, Catalin Marinas  wrote:
> > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> >> to produce new warnings:
> >>
> >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
redefine built-in register 'lr'
> >>
> >> This is
> >>
> >> /*
> >>  * Register aliases.
> >>  */
> >> lr  .reqx30 // link register
> >
> > Strange, does gas now think 'lr' is a general purpose register (aliased
> > to x30)? It never was and IIRC the toolchain people many years ago
> > refused to add it, hence the alias above in the kernel. I wonder if they
> > added 'fp' as well...


I don't remember the reasons why it was refused in the past. Asking 
around I can't seem to find anyone who remembers why either and it would 
be much quicker to fix the problem in binutils than any of this 
archeology :)



> >
> > We could remove the alias and replace all 'lr' instances with 'x30'
> > throughout the kernel (no too many) or we add some #ifdef around the
> > above based on the binutils version.
> 
> This is annoying. Replacing lr with x30 achieves the opposite of the

> intent of the binutils change. And using #ifdefs is inaccurate,
> because you can't really test the binutils version only the GCC
> version, and those are not tightly coupled.
> 
> Can you .unreq it?


Not really, with an older binutils I get:

arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'

I personally consider this a binutils bug. After 6+ years (probably not
all public) of building the kernel just fine, all of a sudden certain
strings became reserved in gas. Three options:


IIRC the ABI states that LR is an alias for X30, FP is an alias for X29,
IP0 an alias for X16 and IP1 an alias for X17.

The warning is unfortunate - looking at the implementation of the
.req directive in gas it's something controlled in the aarch64 backend
and thus we could change gas to make LR (and friends) an internal alias 
thus removing the warning.


Any attempts to realias these to the correct meaning as per the AAPCS 
continues to work from my limited testing of a prototype patch so far.


I'll do a test run and post it on the lists and point you guys to it 
(probably now after lunch as I have a couple of meetings to run to) so 
that you could also test it and let us know if there are any issues.



regards
Ramana


Re: New assembler warnings with binutils 2.29

2017-08-14 Thread Catalin Marinas
On Fri, Aug 11, 2017 at 10:26:06AM +0100, Ard Biesheuvel wrote:
> On 11 August 2017 at 10:22, Catalin Marinas  wrote:
> > On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> >> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> >> to produce new warnings:
> >>
> >> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> >> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
> >> redefine built-in register 'lr'
> >>
> >> This is
> >>
> >> /*
> >>  * Register aliases.
> >>  */
> >> lr  .reqx30 // link register
> >
> > Strange, does gas now think 'lr' is a general purpose register (aliased
> > to x30)? It never was and IIRC the toolchain people many years ago
> > refused to add it, hence the alias above in the kernel. I wonder if they
> > added 'fp' as well...
> >
> > We could remove the alias and replace all 'lr' instances with 'x30'
> > throughout the kernel (no too many) or we add some #ifdef around the
> > above based on the binutils version.
> 
> This is annoying. Replacing lr with x30 achieves the opposite of the
> intent of the binutils change. And using #ifdefs is inaccurate,
> because you can't really test the binutils version only the GCC
> version, and those are not tightly coupled.
> 
> Can you .unreq it?

Not really, with an older binutils I get:

arch/arm64/include/asm/assembler.h:125: Error: unknown register alias 'lr'

I personally consider this a binutils bug. After 6+ years (probably not
all public) of building the kernel just fine, all of a sudden certain
strings became reserved in gas. Three options:

a) binutils reverts the change
b) we stop using lr etc. in Linux for good (since you can't tell which
   gas supports them)
c) we replace .req with #define in the kernel

My preference is (a) but we can go for (c) as being more under our
control.

-- 
Catalin


Re: New assembler warnings with binutils 2.29

2017-08-11 Thread Ard Biesheuvel
On 11 August 2017 at 10:34, Arnd Bergmann  wrote:
> On Fri, Aug 11, 2017 at 11:26 AM, Ard Biesheuvel
>  wrote:
>> On 11 August 2017 at 10:22, Catalin Marinas  wrote:
>>> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
 Fedora rawhide recently upgraded to binutils 2.29 and this seems
 to produce new warnings:

 ./arch/arm64/include/asm/assembler.h: Assembler messages:
 ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
 redefine built-in register 'lr'

 This is

 /*
  * Register aliases.
  */
 lr  .reqx30 // link register
>>>
>>> Strange, does gas now think 'lr' is a general purpose register (aliased
>>> to x30)? It never was and IIRC the toolchain people many years ago
>>> refused to add it, hence the alias above in the kernel. I wonder if they
>>> added 'fp' as well...
>>>
>>> We could remove the alias and replace all 'lr' instances with 'x30'
>>> throughout the kernel (no too many) or we add some #ifdef around the
>>> above based on the binutils version.
>>>
>>
>> This is annoying. Replacing x30 with lr achieves the opposite of the

Of course, I meant replacing lr with x30 in our code.

>> intent of the binutils change. And using #ifdefs is inaccurate,
>> because you can't really test the binutils version only the GCC
>> version, and those are not tightly coupled.
>>
>> Can you .unreq it?
>
> adding the author of the change to cc
>
> https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=62e20ed45e3da5f3ba695e4ee109317668180fe6
>
> There probably was some reasoning behind the change and an
> intended method for using it.
>
> Arnd


Re: New assembler warnings with binutils 2.29

2017-08-11 Thread Arnd Bergmann
On Fri, Aug 11, 2017 at 11:26 AM, Ard Biesheuvel
 wrote:
> On 11 August 2017 at 10:22, Catalin Marinas  wrote:
>> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
>>> Fedora rawhide recently upgraded to binutils 2.29 and this seems
>>> to produce new warnings:
>>>
>>> ./arch/arm64/include/asm/assembler.h: Assembler messages:
>>> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
>>> redefine built-in register 'lr'
>>>
>>> This is
>>>
>>> /*
>>>  * Register aliases.
>>>  */
>>> lr  .reqx30 // link register
>>
>> Strange, does gas now think 'lr' is a general purpose register (aliased
>> to x30)? It never was and IIRC the toolchain people many years ago
>> refused to add it, hence the alias above in the kernel. I wonder if they
>> added 'fp' as well...
>>
>> We could remove the alias and replace all 'lr' instances with 'x30'
>> throughout the kernel (no too many) or we add some #ifdef around the
>> above based on the binutils version.
>>
>
> This is annoying. Replacing x30 with lr achieves the opposite of the
> intent of the binutils change. And using #ifdefs is inaccurate,
> because you can't really test the binutils version only the GCC
> version, and those are not tightly coupled.
>
> Can you .unreq it?

adding the author of the change to cc

https://sourceware.org/git/?p=binutils-gdb.git;a=commitdiff;h=62e20ed45e3da5f3ba695e4ee109317668180fe6

There probably was some reasoning behind the change and an
intended method for using it.

Arnd


Re: New assembler warnings with binutils 2.29

2017-08-11 Thread Ard Biesheuvel
On 11 August 2017 at 10:22, Catalin Marinas  wrote:
> On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
>> Fedora rawhide recently upgraded to binutils 2.29 and this seems
>> to produce new warnings:
>>
>> ./arch/arm64/include/asm/assembler.h: Assembler messages:
>> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
>> redefine built-in register 'lr'
>>
>> This is
>>
>> /*
>>  * Register aliases.
>>  */
>> lr  .reqx30 // link register
>
> Strange, does gas now think 'lr' is a general purpose register (aliased
> to x30)? It never was and IIRC the toolchain people many years ago
> refused to add it, hence the alias above in the kernel. I wonder if they
> added 'fp' as well...
>
> We could remove the alias and replace all 'lr' instances with 'x30'
> throughout the kernel (no too many) or we add some #ifdef around the
> above based on the binutils version.
>

This is annoying. Replacing x30 with lr achieves the opposite of the
intent of the binutils change. And using #ifdefs is inaccurate,
because you can't really test the binutils version only the GCC
version, and those are not tightly coupled.

Can you .unreq it?


Re: New assembler warnings with binutils 2.29

2017-08-11 Thread Catalin Marinas
On Thu, Aug 10, 2017 at 01:13:22PM -0700, Laura Abbott wrote:
> Fedora rawhide recently upgraded to binutils 2.29 and this seems
> to produce new warnings:
> 
> ./arch/arm64/include/asm/assembler.h: Assembler messages:
> ./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to 
> redefine built-in register 'lr'
> 
> This is
> 
> /*
>  * Register aliases.
>  */
> lr  .reqx30 // link register

Strange, does gas now think 'lr' is a general purpose register (aliased
to x30)? It never was and IIRC the toolchain people many years ago
refused to add it, hence the alias above in the kernel. I wonder if they
added 'fp' as well...

We could remove the alias and replace all 'lr' instances with 'x30'
throughout the kernel (no too many) or we add some #ifdef around the
above based on the binutils version.

-- 
Catalin


New assembler warnings with binutils 2.29

2017-08-10 Thread Laura Abbott
Fedora rawhide recently upgraded to binutils 2.29 and this seems
to produce new warnings:

./arch/arm64/include/asm/assembler.h: Assembler messages:
./arch/arm64/include/asm/assembler.h:125: Warning: ignoring attempt to redefine 
built-in register 'lr'

This is

/*
 * Register aliases.
 */
lr  .reqx30 // link register

https://kojipkgs.fedoraproject.org//packages/kernel/4.13.0/0.rc2.git2.1.fc27/data/logs/aarch64/build.log
has an example log. As far as I can tell this doesn't affect anything at 
runtime.

Any ideas?

Thanks,
Laura