[Bug target/46208] New: TARGET_PROMOTE_PROTOTYPES for SPARC V8

[Bug target/46208] New: TARGET_PROMOTE_PROTOTYPES for SPARC V8

[jorge.perez at invia dot fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

           Summary: TARGET_PROMOTE_PROTOTYPES for SPARC V8
           Product: gcc
           Version: 4.5.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jorge.perez@invia.fr


Based on Ian Taylor's recommendation (http://goo.gl/yAoL)


******************* Problem summary (http://goo.gl/Mcrm)

When a caller function calls a callee function with short or char
arguments, the arguments are casted twice: inside the caller function
and inside the callee function, see the example. It is a waste of
performance in code density and speed!

Example: sparc-elf-gcc -o test.elf test.c -Os

short somme(char a, short b){
  int i;
  for (i=0; i<b; i++){
    a+=a;
  }
  return a+b;
}
short somme2(char a, short b){
  int i;
  for (i=0; i<b; i++){
    a+=b;
  }
  return a+2*b;
}
int main(){
  volatile short b=1;
  volatile char a=1, c=1;
  b=somme(a,b);
  b=somme(c,b);
  b=somme(a,c);
  b=somme2(a,b);
  b=somme2(c,b);
  b=somme2(a,c);
  return 0;
}

0001024c <somme>:
   1024c:    85 2a 60 10     sll  %o1, 0x10, %g2
   10250:    82 10 20 00     clr  %g1
   10254:    10 80 00 03     b  10260 <somme+0x14>
   10258:    85 38 a0 10     sra  %g2, 0x10, %g2
   1025c:    82 00 60 01     inc  %g1
   10260:    80 a0 40 02     cmp  %g1, %g2
   10264:    26 bf ff fe     bl,a   1025c <somme+0x10>
   10268:    91 2a 20 01     sll  %o0, 1, %o0
   1026c:    91 2a 20 18     sll  %o0, 0x18, %o0
   10270:    91 3a 20 18     sra  %o0, 0x18, %o0
   10274:    81 c3 e0 08     retl
   10278:    90 02 40 08     add  %o1, %o0, %o0

000102b4 <main>:
   102b4:    9d e3 bf 98     save  %sp, -104, %sp
   102b8:    82 10 20 01     mov  1, %g1
   102bc:    c2 37 bf fc     sth  %g1, [ %fp + -4 ]
   102c0:    c2 2f bf ff     stb  %g1, [ %fp + -1 ]
   102c4:    c2 2f bf fe     stb  %g1, [ %fp + -2 ]
   102c8:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   102cc:    d2 17 bf fc     lduh  [ %fp + -4 ], %o1
   102d0:    91 2a 20 18     sll  %o0, 0x18, %o0
   102d4:    93 2a 60 10     sll  %o1, 0x10, %o1
   102d8:    91 3a 20 18     sra  %o0, 0x18, %o0
   102dc:    7f ff ff dc     call  1024c <somme>
   102e0:    93 3a 60 10     sra  %o1, 0x10, %o1
      ...


**************** Fix proposal (http://goo.gl/0mJ1)

we modified the function *TARGET_PROMOTE_PROTOTYPES
*(http://goo.gl/2pQQ) and set it to the default value which is FALSE.

static bool
sparc_promote_prototypes (const_tree fntype ATTRIBUTE_UNUSED)
{
  return TARGET_ARCH32 ? true : false;
}

was replaced by

static bool
sparc_promote_prototypes (const_tree fntype ATTRIBUTE_UNUSED)
{
  return false;
}

Then we re-compiled GCC 4.5.1 for SPARC V8 and the results for the
previously mentioned test.c are

short somme(char a, short b){
  int i;
  for (i=0; i<b; i++){
    a+=a;
  }
  return a+b;
}

short somme2(char a, short b){
  int i;
  for (i=0; i<b; i++){
    a+=b;
  }
  return a+2*b;
}

int main(){
  volatile short b=1;
  volatile char a=1, c=1;
  b=somme(a,b);
  b=somme(c,b);
  b=somme(a,c);
  b=somme2(a,b);
  b=somme2(c,b);
  b=somme2(a,c);
  b=somme2(a,c*2);
  b=somme2(a,c*3);
  b=somme2(a,c*4);      
  return 0;
}

0001024c <somme>:
   1024c:    85 2a 60 10     sll  %o1, 0x10, %g2
   10250:    82 10 20 00     clr  %g1
   10254:    10 80 00 03     b  10260 <somme+0x14>
   10258:    85 38 a0 10     sra  %g2, 0x10, %g2
   1025c:    82 00 60 01     inc  %g1
   10260:    80 a0 40 02     cmp  %g1, %g2
   10264:    26 bf ff fe     bl,a   1025c <somme+0x10>
   10268:    91 2a 20 01     sll  %o0, 1, %o0
   1026c:    91 2a 20 18     sll  %o0, 0x18, %o0
   10270:    91 3a 20 18     sra  %o0, 0x18, %o0
   10274:    81 c3 e0 08     retl
   10278:    90 02 00 09     add  %o0, %o1, %o0

0001027c <somme2>:
   1027c:    87 2a 60 10     sll  %o1, 0x10, %g3
   10280:    82 10 20 00     clr  %g1
   10284:    87 38 e0 10     sra  %g3, 0x10, %g3
   10288:    10 80 00 03     b  10294 <somme2+0x18>
   1028c:    84 10 00 09     mov  %o1, %g2
   10290:    82 00 60 01     inc  %g1
   10294:    80 a0 40 03     cmp  %g1, %g3
   10298:    26 bf ff fe     bl,a   10290 <somme2+0x14>
   1029c:    90 02 00 02     add  %o0, %g2, %o0
   102a0:    93 2a 60 01     sll  %o1, 1, %o1
   102a4:    91 2a 20 18     sll  %o0, 0x18, %o0
   102a8:    91 3a 20 18     sra  %o0, 0x18, %o0
   102ac:    81 c3 e0 08     retl
   102b0:    90 02 40 08     add  %o1, %o0, %o0

000102b4 <main>:
   102b4:    9d e3 bf 98     save  %sp, -104, %sp
   102b8:    82 10 20 01     mov  1, %g1
   102bc:    c2 37 bf fc     sth  %g1, [ %fp + -4 ]
   102c0:    c2 2f bf ff     stb  %g1, [ %fp + -1 ]
   102c4:    c2 2f bf fe     stb  %g1, [ %fp + -2 ]
   102c8:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   102cc:    d2 17 bf fc     lduh  [ %fp + -4 ], %o1
   102d0:    7f ff ff df     call  1024c <somme>
   102d4:    01 00 00 00     nop
   102d8:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   102dc:    d0 0f bf fe     ldub  [ %fp + -2 ], %o0
   102e0:    d2 17 bf fc     lduh  [ %fp + -4 ], %o1
   102e4:    7f ff ff da     call  1024c <somme>
   102e8:    01 00 00 00     nop
   102ec:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   102f0:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   102f4:    d2 0f bf fe     ldub  [ %fp + -2 ], %o1
   102f8:    93 2a 60 18     sll  %o1, 0x18, %o1
   102fc:    7f ff ff d4     call  1024c <somme>
   10300:    93 3a 60 18     sra  %o1, 0x18, %o1
   10304:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   10308:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   1030c:    d2 17 bf fc     lduh  [ %fp + -4 ], %o1
   10310:    7f ff ff db     call  1027c <somme2>
   10314:    01 00 00 00     nop
   10318:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   1031c:    d0 0f bf fe     ldub  [ %fp + -2 ], %o0
   10320:    d2 17 bf fc     lduh  [ %fp + -4 ], %o1
   10324:    7f ff ff d6     call  1027c <somme2>
   10328:    01 00 00 00     nop
   1032c:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   10330:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   10334:    d2 0f bf fe     ldub  [ %fp + -2 ], %o1
   10338:    93 2a 60 18     sll  %o1, 0x18, %o1
   1033c:    7f ff ff d0     call  1027c <somme2>
   10340:    93 3a 60 18     sra  %o1, 0x18, %o1
   10344:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   10348:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   1034c:    d2 0f bf fe     ldub  [ %fp + -2 ], %o1
   10350:    93 2a 60 18     sll  %o1, 0x18, %o1
   10354:    7f ff ff ca     call  1027c <somme2>
   10358:    93 3a 60 17     sra  %o1, 0x17, %o1
   1035c:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   10360:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   10364:    c2 0f bf fe     ldub  [ %fp + -2 ], %g1
   10368:    83 28 60 18     sll  %g1, 0x18, %g1
   1036c:    83 38 60 18     sra  %g1, 0x18, %g1
   10370:    93 28 60 01     sll  %g1, 1, %o1
   10374:    7f ff ff c2     call  1027c <somme2>
   10378:    92 02 40 01     add  %o1, %g1, %o1
   1037c:    b0 10 20 00     clr  %i0
   10380:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   10384:    d0 0f bf ff     ldub  [ %fp + -1 ], %o0
   10388:    d2 0f bf fe     ldub  [ %fp + -2 ], %o1
   1038c:    93 2a 60 18     sll  %o1, 0x18, %o1
   10390:    7f ff ff bb     call  1027c <somme2>
   10394:    93 3a 60 16     sra  %o1, 0x16, %o1
   10398:    d0 37 bf fc     sth  %o0, [ %fp + -4 ]
   1039c:    81 c7 e0 08     ret
   103a0:    81 e8 00 00     restore


this reduced the size of the main by 96 bytes which is huge taking into
account that this is a dummy example. 

Thanks for your help,


Best regards,


Jorge Perez

[Bug target/46208] TARGET_PROMOTE_PROTOTYPES for SPARC V8

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org

--- Comment #1 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-10-28 10:57:12 UTC ---
> we modified the function *TARGET_PROMOTE_PROTOTYPES
> *(http://goo.gl/2pQQ) and set it to the default value which is FALSE.
> 
> static bool
> sparc_promote_prototypes (const_tree fntype ATTRIBUTE_UNUSED)
> {
>   return TARGET_ARCH32 ? true : false;
> }
> 
> was replaced by
> 
> static bool
> sparc_promote_prototypes (const_tree fntype ATTRIBUTE_UNUSED)
> {
>   return false;
> }

This will result in violations of the ABI without further changes (garbage in
the upper part of the argument registers).  You need the patch I'm about to
attach as a minimum, but it cannot eliminate the extensions in the caller
alone, more work is required in the back-end.  At which point changing
TARGET_PROMOTE_PROTOTYPES will probably have only a marginal effect.

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |sparc-*-*
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.10.28 10:58:45
            Summary|TARGET_PROMOTE_PROTOTYPES   |redundant extensions of
                   |for SPARC V8                |argument registers
     Ever Confirmed|0                           |1
           Severity|normal                      |enhancement

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-10-28 11:02:59 UTC ---
Created attachment 22188
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22188
Minimal patch

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #22188|0                           |1
        is obsolete|                            |

--- Comment #3 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-03 00:18:51 UTC ---
Created attachment 22243
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22243
Revised minimal patch

Lightly tested.

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #4 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-08 12:15:02 UTC ---
Author: ebotcazou
Date: Mon Nov  8 12:14:51 2010
New Revision: 166433

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166433
Log:
    PR target/46208
    * config/sparc/sparc.c (TARGET_PROMOTE_PROTOTYPES): Delete.
    (sparc_promote_prototypes): Likewise.
    (sparc_promote_function_mode): Promote in 32-bit mode as well.
    (sparc_return_in_memory): Remove superfluous parentheses.
    (sparc_struct_value_rtx): Fix long lines.
    (sparc_function_value_1): Promote in 32-bit mode as well.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/sparc/sparc.c

[Bug target/46208] redundant extensions of argument registers

[pinskia at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> 2010-11-08 16:04:18 UTC ---
(In reply to comment #4)
>     * config/sparc/sparc.c (TARGET_PROMOTE_PROTOTYPES): Delete.

I was just looking into the same problem with MIPS too. I almost think
TARGET_PROMOTE_PROTOTYPES in all targets be removed but that is up to each
target maintainer.

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-08 17:46:31 UTC ---
> I was just looking into the same problem with MIPS too. I almost think
> TARGET_PROMOTE_PROTOTYPES in all targets be removed but that is up to each
> target maintainer.

Yes, this should make sense with sane ABIs.  But IIRC the ARM has a comment
saying that it cannot return false unconditionally.

[Bug target/46208] redundant extensions of argument registers

[jorge.perez at invia dot fr]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #7 from Jorge Perez <jorge.perez at invia dot fr> 2010-11-15 12:29:51 UTC ---
Hello Eric,

With a colleague we've been trying to do some tests with this patch but due to
differences in the versions you and we are working on we couldn't just apply
the patch. So we checked out the rev. 166433 that is when the patch was
introduced to the trunk.
However during the GCC compilation we got the following message while executing
the make all command (there is no error for make all-gcc):

------------------------------------------------------------
configure: error: Link tests are not allowed after GCC_NO_EXECUTABLES.
make[3]: *** [configure-target-libiberty] Error 1
------------------------------------------------------------

in Google this seems a recurrent error for several targets, although we don't
know how to fix this, can you give us a hand with this please? it's the first
time we see the error during 4.5.1 compilation.

FYI the GCC configuration is the following:
$(BASE_DIR)/gcc-trunk/configure --target=sparc-elf --prefix=$(BASE_DIR)/out \
--with-float=soft --with-cpu=v8 --disable-multilib --enable-lto
--enable-languages="c" --with-newlib \
--with-headers=$(BASE_DIR)/newlib-1.13.0/newlib/libc/include \
--disable-shared --disable-nls --with-gnu-as --with-gnu-ld


(In reply to comment #4)
> Author: ebotcazou
> Date: Mon Nov  8 12:14:51 2010
> New Revision: 166433
> 
> URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=166433
> Log:
>     PR target/46208
>     * config/sparc/sparc.c (TARGET_PROMOTE_PROTOTYPES): Delete.
>     (sparc_promote_prototypes): Likewise.
>     (sparc_promote_function_mode): Promote in 32-bit mode as well.
>     (sparc_return_in_memory): Remove superfluous parentheses.
>     (sparc_struct_value_rtx): Fix long lines.
>     (sparc_function_value_1): Promote in 32-bit mode as well.
> 
> Modified:
>     trunk/gcc/ChangeLog
>     trunk/gcc/config/sparc/sparc.c

[Bug target/46208] redundant extensions of argument registers

[ebotcazou at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=46208

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2010-11-19 07:50:34 UTC ---
> However during the GCC compilation we got the following message while executing
> the make all command (there is no error for make all-gcc):
> 
> ------------------------------------------------------------
> configure: error: Link tests are not allowed after GCC_NO_EXECUTABLES.
> make[3]: *** [configure-target-libiberty] Error 1
> ------------------------------------------------------------

See http://gcc.gnu.org/ml/gcc-patches/2010-11/msg01958.html