Re: [PATCH] fix BZ 18116 - build failure on ppc64le: setcontext.S uses power6 mtfsf when not supported

["Carlos O'Donell" <carlos at redhat dot com>]

On 03/16/2015 01:41 PM, Martin Sebor wrote:
> The attached patch fixes a glibc build failure with gcc 5 on
> powerpc64le caused by a recent change in gcc where the compiler
> defines the _ARCH_PWR6 macro when processing assembly files
> but doesn't invoke the assembler in the corresponding machine
> mode (unless it has been explicitly configured to target POWER
> 6 or later). A bug had been filed with gcc for this (65341) but
> was closed as won't fix. Glibc relies on the _ARCH_PWR6 macro
> in a few .S files to make use of Power ISA 2.5 instructions
> (specifically, the four-argument form of the mtfsf insn).
> A similar problem had occurred in the past (bug 10118) but
> the fix that was committed for it didn't anticipate this new
> problem. The fix in the proposed patch introduces the .machine
> "power6" directive unconditionaly, regardless of whether
> _ARCH_PWR6 is defined.

For avoidance of doubt the `#ifdef _ARCH_PWR6` need cleaning up,
but that's another issue orthogonal to this patch. If gcc no longer
defines _ARCH_PWR6 when invoking the assembler, then glibc has use
it's own configury to determine exactly which of the two code
segments to include in the final assembly.

Given that both branches of the #ifdef use mtfsf they both require
power6 or higher, and therefore it is correct to move the .machine
directives outside of the #ifdef.

If you tested this worked on pp64 and ppc64le, then I'd say this
is an obvious fix that you can commit immediately without waiting
for Steven or Tulio to comment (machine maintainers).

Cheers,
Carlos.

> 2015-03-11  Martin Sebor  <msebor@redhat.com>
> 
>         * sysdeps/unix/sysv/linux/powerpc/powerpc64/setcontext.S
>         (__setcontext): Set machine to power6 regardless of whether
>         or not _ARCH_PWR6 is defined.
>         * sysdeps/unix/sysv/linux/powerpc/powerpc64/swapcontext.S
>         (__novec_swapcontext): Likewise.
> 
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/setcontext.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/setcontext.S
> index e47a57a..a1ed419 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/setcontext.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/setcontext.S
> @@ -79,12 +79,13 @@ ENTRY(__novec_setcontext)
>    lfd  fp31,(SIGCONTEXT_FP_REGS+(PT_R31*8))(r31)
>    lfd  fp30,(SIGCONTEXT_FP_REGS+(PT_R30*8))(r31)
>  
> +  .machine push
> +  .machine "power6"
> +
>  # ifdef _ARCH_PWR6
>    /* Use the extended four-operand version of the mtfsf insn.  */
>    mtfsf  0xff,fp0,1,0
>  # else
> -  .machine push
> -  .machine "power6"
>    /* Availability of DFP indicates a 64-bit FPSCR.  */
>    andi.  r6,r5,PPC_FEATURE_HAS_DFP
>    beq    5f
> @@ -95,8 +96,10 @@ ENTRY(__novec_setcontext)
>  5:
>    mtfsf  0xff,fp0
>  6:
> -  .machine pop
>  # endif /* _ARCH_PWR6 */
> +
> +  .machine pop
> +
>    lfd  fp29,(SIGCONTEXT_FP_REGS+(PT_R29*8))(r31)
>    lfd  fp28,(SIGCONTEXT_FP_REGS+(PT_R28*8))(r31)
>    lfd  fp27,(SIGCONTEXT_FP_REGS+(PT_R27*8))(r31)
> @@ -362,12 +365,13 @@ L(has_no_vec):
>    lfd  fp31,(SIGCONTEXT_FP_REGS+(PT_R31*8))(r31)
>    lfd  fp30,(SIGCONTEXT_FP_REGS+(PT_R30*8))(r31)
>  
> +  .machine push
> +  .machine "power6"
> +
>  # ifdef _ARCH_PWR6
>    /* Use the extended four-operand version of the mtfsf insn.  */
>    mtfsf  0xff,fp0,1,0
>  # else
> -  .machine push
> -  .machine "power6"
>    /* Availability of DFP indicates a 64-bit FPSCR.  */
>    andi.  r6,r5,PPC_FEATURE_HAS_DFP
>    beq    7f
> @@ -378,8 +382,10 @@ L(has_no_vec):
>  7:
>    mtfsf  0xff,fp0
>  8:
> -  .machine pop
>  # endif /* _ARCH_PWR6 */
> +
> +  .machine pop
> +
>    lfd  fp29,(SIGCONTEXT_FP_REGS+(PT_R29*8))(r31)
>    lfd  fp28,(SIGCONTEXT_FP_REGS+(PT_R28*8))(r31)
>    lfd  fp27,(SIGCONTEXT_FP_REGS+(PT_R27*8))(r31)
> diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/swapcontext.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/swapcontext.S
> index bc02a21..b25904d 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/swapcontext.S
> +++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/swapcontext.S
> @@ -173,6 +173,10 @@ ENTRY(__novec_swapcontext)
>    lfd  fp0,(SIGCONTEXT_FP_REGS+(32*8))(r31)
>    lfd  fp31,(SIGCONTEXT_FP_REGS+(PT_R31*8))(r31)
>    lfd  fp30,(SIGCONTEXT_FP_REGS+(PT_R30*8))(r31)
> +
> +  .machine push
> +  .machine "power6"
> +        
>  # ifdef _ARCH_PWR6
>    /* Use the extended four-operand version of the mtfsf insn.  */
>    mtfsf  0xff,fp0,1,0
> @@ -189,8 +193,10 @@ ENTRY(__novec_swapcontext)
>  5:
>    mtfsf  0xff,fp0
>  6:
> -  .machine pop
>  #endif /* _ARCH_PWR6 */
> +
> +  .machine pop
> +
>    lfd  fp29,(SIGCONTEXT_FP_REGS+(PT_R29*8))(r31)
>    lfd  fp28,(SIGCONTEXT_FP_REGS+(PT_R28*8))(r31)
>    lfd  fp27,(SIGCONTEXT_FP_REGS+(PT_R27*8))(r31)
> @@ -652,12 +658,14 @@ L(has_no_vec2):
>    lfd  fp0,(SIGCONTEXT_FP_REGS+(32*8))(r31)
>    lfd  fp31,(SIGCONTEXT_FP_REGS+(PT_R31*8))(r31)
>    lfd  fp30,(SIGCONTEXT_FP_REGS+(PT_R30*8))(r31)
> +
> +  .machine push
> +  .machine "power6"
> +
>  # ifdef _ARCH_PWR6
>    /* Use the extended four-operand version of the mtfsf insn.  */
>    mtfsf  0xff,fp0,1,0
>  # else
> -  .machine push
> -  .machine "power6"
>    /* Availability of DFP indicates a 64-bit FPSCR.  */
>    andi.  r6,r8,PPC_FEATURE_HAS_DFP
>    beq    7f
> @@ -668,8 +676,10 @@ L(has_no_vec2):
>  7:
>    mtfsf  0xff,fp0
>  8:
> -  .machine pop
>  #endif /* _ARCH_PWR6 */
> +
> +  .machine pop
> +
>    lfd  fp29,(SIGCONTEXT_FP_REGS+(PT_R29*8))(r31)
>    lfd  fp28,(SIGCONTEXT_FP_REGS+(PT_R28*8))(r31)
>    lfd  fp27,(SIGCONTEXT_FP_REGS+(PT_R27*8))(r31)