[Pixman] [PATCH] MIPS: Use .if instead of #ifdef definition of LEAF_MIPS32R2

[Siarhei Siamashka]

On Tue,  5 May 2015 13:13:31 +0100
James Cowgill <james410 at cowgill.org.uk> wrote:

> From: Vicente Olivert Riera <Vincent.Riera at imgtec.com>
> 
> Commit 6d2cf40166d8 ("MIPS: Fix exported symbols in public API") attempted to
> add a .hidden assembly directive, conditional on the code being compiled for an
> ELF target. Unfortunately the #ifdef added was already inside a macro and
> wasn't expanded properly by the preprocessor.
> 
> Fix by using an assembly .if directive which is handled by the assembler later
> on.
> 
> Fixes: Bug 83358 (https://bugs.freedesktop.org/83358)
> Fixes: 6d2cf40166d8 ("MIPS: Fix exported symbols in public API")
> Signed-off-by: Vicente Olivert Riera <Vincent.Riera at imgtec.com>
> Signed-off-by: James Cowgill <james410 at cowgill.org.uk>
> Cc: Nemanja Lukic <nemanja.lukic at rt-rk.com>
> ---
>  pixman/pixman-mips-dspr2-asm.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/pixman/pixman-mips-dspr2-asm.h b/pixman/pixman-mips-dspr2-asm.h
> index 11849bd..b2696d0 100644
> --- a/pixman/pixman-mips-dspr2-asm.h
> +++ b/pixman/pixman-mips-dspr2-asm.h
> @@ -72,10 +72,10 @@
>  #define LEAF_MIPS32R2(symbol)                           \
>                  .globl  symbol;                         \
>                  .align  2;                              \
> -#ifdef __ELF__
> +            .if __ELF__;                                \
>                  .hidden symbol;                         \
>                  .type   symbol, @function;              \
> -#endif
> +            .endif;                                     \
>                  .ent    symbol, 0;                      \
>  symbol:         .frame  sp, 0, ra;                      \
>                  .set    push;                           \

Thanks. But this is not quite correct. For example, try the
following example with the GNU assembler:

    $ cat test.S
    .if __FOOBAR__
       .foobar_specific_directive
    .endif

    $ gcc test.S
    test.S: Assembler messages:
    test.S:1: Error: non-constant expression in ".if" statement

As such, the ".if __ELF__" code will fail to compile on non-ELF systems.

I would suggest to simply remove this ".if __ELF__" / "#ifdef __ELF__"
check altogether. There is probably no notable MIPS system using a
non-ELF object file format nowadays. And even if such a MIPS system
exists, its users will just get a more meaningful error message about
the unsupported ELF-specific ".hidden" directive, which would look
similar to:

    test.S:1: Error: unknown pseudo-op: `.foobar_specific_directive'

-- 
Best regards,
Siarhei Siamashka