[RFC,AArch64] Implement TARGET_PROMOTE_FUNCTION_MODE for ILP32 code generation

Hi,

  For this trivial testcase: 

extern int bar (int , int);

int foo (int *a, int *b)
{
    return bar (*a, *b);
}

I noticed that GCC generate redundant zero-extension instructions under ILP32 (aarch64-linux-gnu-gcc -S -O2 -mabi=ilp32).
Assembly code:
        .arch armv8-a+fp+simd
        .file   "1.c"
        .text
        .align  2
        .p2align 3,,7
        .global foo
        .type   foo, %function
foo:
        uxtw    x0, w0
        uxtw    x1, w1
        ldr     w0, [x0]
        ldr     w1, [x1]
        b       bar
        .size   foo, .-foo
        .ident  "GCC: (20140403) 5.0.0 20150312 (experimental)"

According the ILP32 ABI, the two "uxtw" instrunctions here is not necessary.
The following is a proposed patch to fix this issue, i.e. promoting pointer function arguments to word_mode.
But I don't know whether it's a good idea to do this for pointer return values.
Any comments?

> On Mar 16, 2015, at 2:28 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:
> 
> Hi,
> 
>  For this trivial testcase: 
> 
> extern int bar (int , int);
> 
> int foo (int *a, int *b)
> {
>    return bar (*a, *b);
> }
> 
> I noticed that GCC generate redundant zero-extension instructions under ILP32 (aarch64-linux-gnu-gcc -S -O2 -mabi=ilp32).
> Assembly code:
>        .arch armv8-a+fp+simd
>        .file   "1.c"
>        .text
>        .align  2
>        .p2align 3,,7
>        .global foo
>        .type   foo, %function
> foo:
>        uxtw    x0, w0
>        uxtw    x1, w1
>        ldr     w0, [x0]
>        ldr     w1, [x1]
>        b       bar
>        .size   foo, .-foo
>        .ident  "GCC: (20140403) 5.0.0 20150312 (experimental)"
> 
> According the ILP32 ABI, the two "uxtw" instrunctions here is not necessary.
> The following is a proposed patch to fix this issue, i.e. promoting pointer function arguments to word_mode.
> But I don't know whether it's a good idea to do this for pointer return values.
> Any comments?


Actually they are required. The abi says the upper 32bits are undefined for arguments smaller then 64bits. I had this discussion a year or more ago about this case. 

A simple struct like
struct a { int * b; int c; }; 

Will break the code if we don't have the zero extends

Try 
void f(int *);
void g(struct a d)
{
  f(d.b);
}

And see that there is no zero extends inside g.  I saw this exact thing when working on getting gccgo working.

It also means the assembly functions in glibc are broken and need to be fixed.

Thanks,
Andrew
> 
> 
> Index: gcc/config/aarch64/aarch64.c
> ===================================================================
> --- gcc/config/aarch64/aarch64.c    (revision 221393)
> +++ gcc/config/aarch64/aarch64.c    (working copy)
> @@ -1587,7 +1587,7 @@ aarch64_function_value (const_tree type, const_tre
>   machine_mode ag_mode;
> 
>   mode = TYPE_MODE (type);
> -  if (INTEGRAL_TYPE_P (type))
> +  if (INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type))
>     mode = promote_function_mode (type, mode, &unsignedp, func, 1);
> 
>   if (aarch64_return_in_msb (type))
> @@ -1650,6 +1650,24 @@ aarch64_function_value_regno_p (const unsigned int
>   return false;
> }
> 
> +/* Implement TARGET_PROMOTE_FUNCTION_MODE.  */
> +
> +static machine_mode
> +aarch64_promote_function_mode (const_tree type, machine_mode mode,
> +                   int *punsignedp, const_tree fntype,
> +                   int for_return)
> +{
> +  /* Pointer function arguments and return values are promoted to word_mode.  */
> +  if (type != NULL_TREE && POINTER_TYPE_P (type))
> +    {
> +      *punsignedp = POINTERS_EXTEND_UNSIGNED;
> +      return word_mode;
> +    }
> +
> +  return default_promote_function_mode (type, mode, punsignedp, fntype,
> +                                        for_return);
> +}
> +
> /* Implement TARGET_RETURN_IN_MEMORY.
> 
>    If the type T of the result of a function is such that
> @@ -11329,6 +11347,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool l
> #define TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE \
>   aarch64_override_options_after_change
> 
> +#undef TARGET_PROMOTE_FUNCTION_MODE
> +#define TARGET_PROMOTE_FUNCTION_MODE aarch64_promote_function_mode
> +
> #undef TARGET_PASS_BY_REFERENCE
> #define TARGET_PASS_BY_REFERENCE aarch64_pass_by_reference
> <aarch64-promote-v2.diff>

Hi Andrew, 

    Sorry for the late reply.  Seems that I misunderstood the AAPCS64 specification.  
    Thanks for the clarification.  


> 

> > On Mar 16, 2015, at 2:28 AM, Yangfei (Felix) <felix.yang@huawei.com> wrote:

> >

> > Hi,

> >

> >  For this trivial testcase:

> >

> > extern int bar (int , int);

> >

> > int foo (int *a, int *b)

> > {

> >    return bar (*a, *b);

> > }

> >

> > I noticed that GCC generate redundant zero-extension instructions under ILP32

> (aarch64-linux-gnu-gcc -S -O2 -mabi=ilp32).

> > Assembly code:

> >        .arch armv8-a+fp+simd

> >        .file   "1.c"

> >        .text

> >        .align  2

> >        .p2align 3,,7

> >        .global foo

> >        .type   foo, %function

> > foo:

> >        uxtw    x0, w0

> >        uxtw    x1, w1

> >        ldr     w0, [x0]

> >        ldr     w1, [x1]

> >        b       bar

> >        .size   foo, .-foo

> >        .ident  "GCC: (20140403) 5.0.0 20150312 (experimental)"

> >

> > According the ILP32 ABI, the two "uxtw" instrunctions here is not necessary.

> > The following is a proposed patch to fix this issue, i.e. promoting pointer

> function arguments to word_mode.

> > But I don't know whether it's a good idea to do this for pointer return values.

> > Any comments?

> 

> 

> Actually they are required. The abi says the upper 32bits are undefined for

> arguments smaller then 64bits. I had this discussion a year or more ago about this

> case.

> 

> A simple struct like

> struct a { int * b; int c; };

> 

> Will break the code if we don't have the zero extends

> 

> Try

> void f(int *);

> void g(struct a d)

> {

>   f(d.b);

> }

> 

> And see that there is no zero extends inside g.  I saw this exact thing when

> working on getting gccgo working.

> 

> It also means the assembly functions in glibc are broken and need to be fixed.

> 

> Thanks,

> Andrew

> >

> >

> > Index: gcc/config/aarch64/aarch64.c

> >

> =============================================================

> ======

> > --- gcc/config/aarch64/aarch64.c    (revision 221393)

> > +++ gcc/config/aarch64/aarch64.c    (working copy)

> > @@ -1587,7 +1587,7 @@ aarch64_function_value (const_tree type, const_tre

> >   machine_mode ag_mode;

> >

> >   mode = TYPE_MODE (type);

> > -  if (INTEGRAL_TYPE_P (type))

> > +  if (INTEGRAL_TYPE_P (type) || POINTER_TYPE_P (type))

> >     mode = promote_function_mode (type, mode, &unsignedp, func, 1);

> >

> >   if (aarch64_return_in_msb (type))

> > @@ -1650,6 +1650,24 @@ aarch64_function_value_regno_p (const unsigned

> int

> >   return false;

> > }

> >

> > +/* Implement TARGET_PROMOTE_FUNCTION_MODE.  */

> > +

> > +static machine_mode

> > +aarch64_promote_function_mode (const_tree type, machine_mode mode,

> > +                   int *punsignedp, const_tree fntype,

> > +                   int for_return)

> > +{

> > +  /* Pointer function arguments and return values are promoted to

> > +word_mode.  */

> > +  if (type != NULL_TREE && POINTER_TYPE_P (type))

> > +    {

> > +      *punsignedp = POINTERS_EXTEND_UNSIGNED;

> > +      return word_mode;

> > +    }

> > +

> > +  return default_promote_function_mode (type, mode, punsignedp, fntype,

> > +                                        for_return); }

> > +

> > /* Implement TARGET_RETURN_IN_MEMORY.

> >

> >    If the type T of the result of a function is such that @@ -11329,6

> > +11347,9 @@ aarch64_gen_adjusted_ldpstp (rtx *operands, bool l #define

> > TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE \

> >   aarch64_override_options_after_change

> >

> > +#undef TARGET_PROMOTE_FUNCTION_MODE

> > +#define TARGET_PROMOTE_FUNCTION_MODE

> aarch64_promote_function_mode

> > +

> > #undef TARGET_PASS_BY_REFERENCE

> > #define TARGET_PASS_BY_REFERENCE aarch64_pass_by_reference

> > <aarch64-promote-v2.diff>