Bug 44392 - [4.5 Regression] libgcc compile with --enable-target-optspace (-Os) causes recursion in __bswapsi2
Summary: [4.5 Regression] libgcc compile with --enable-target-optspace (-Os) causes re...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.5.1
: P2 normal
Target Milestone: 4.5.3
Assignee: Not yet assigned to anyone
URL:
Keywords: patch, wrong-code
Depends on:
Blocks:
 
Reported: 2010-06-02 18:57 UTC by Khem Raj
Modified: 2011-01-26 10:44 UTC (History)
5 users (show)

See Also:
Host: x86_64-linux-gnu
Target: arm-oe-linux-gnueabi
Build: x86_64-linux-gnu
Known to work: 4.4.4, 4.6.0
Known to fail: 4.5.0
Last reconfirmed: 2010-06-22 00:46:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Khem Raj 2010-06-02 18:57:23 UTC
When using --enable-target-optspace on gcc 4.5 function __bswapsi2 in libgcc generates call to itself which causes infinite recursion. Here is disassembly.

$ arm-none-linux-uclibcgnueabi-objdump -d _bswapsi2_s.o 

_bswapsi2_s.o:     file format elf32-littlearm


Disassembly of section .text:

00000000 <__bswapsi2>:
   0:   e92d4008        push    {r3, lr}
   4:   ebfffffe        bl      0 <__bswapsi2>
   8:   e8bd8008        pop     {r3, pc}


This does not happen on -O1, O2 or O3 where is computes the swapping
and does not call the libgcc function. With gcc 4.4 it does not call
libgcc function even at -Os. I have only tested it on arm architecture.

Here is little program which demonstrates it

typedef signed int SItype;
SItype
__bswapsi2 (SItype u)
{
  return ((((u) & 0xff000000) >> 24)
          | (((u) & 0x00ff0000) >>  8)
          | (((u) & 0x0000ff00) <<  8)
          | (((u) & 0x000000ff) << 24));
}


here is how the gcc was built

/scratch/oe/work/armv5te-oe-linux-gnueabi/gcc-cross-intermediate-4.5-r0+svnr160043/gcc-4.5/build.x86_64-linux.arm-oe-linux-gnueabi/gcc/xgcc -v
Using built-in specs.
COLLECT_GCC=/scratch/oe/work/armv5te-oe-linux-gnueabi/gcc-cross-intermediate-4.5-r0+svnr160043/gcc-4.5/build.x86_64-linux.arm-oe-linux-gnueabi/gcc/xgcc
Target: arm-oe-linux-gnueabi
Configured with: /scratch/oe/work/armv5te-oe-linux-gnueabi/gcc-cross-intermediate-4.5-r0+svnr160043/gcc-4.5/configure --build=x86_64-linux --host=x86_64-linux --target=arm-oe-linux-gnueabi --prefix=/scratch/oe/cross/armv5te --exec_prefix=/scratch/oe/cross/armv5te --bindir=/scratch/oe/cross/armv5te/bin --sbindir=/scratch/oe/cross/armv5te/bin --libexecdir=/scratch/oe/cross/armv5te/libexec --datadir=/scratch/oe/cross/armv5te/share --sysconfdir=/scratch/oe/cross/armv5te/etc --sharedstatedir=/scratch/oe/cross/armv5te/com --localstatedir=/scratch/oe/cross/armv5te/var --libdir=/scratch/oe/cross/armv5te/lib --includedir=/scratch/oe/cross/armv5te/include --oldincludedir=/scratch/oe/cross/armv5te/include --infodir=/scratch/oe/cross/armv5te/share/info --mandir=/scratch/oe/cross/armv5te/share/man --with-local-prefix=/scratch/oe/sysroots/armv5te-oe-linux-gnueabi/usr --enable-shared --disable-multilib --disable-threads --enable-languages=c --program-prefix=arm-oe-linux-gnueabi- --with-sysroot=/scratch/oe/sysroots/armv5te-oe-linux-gnueabi --with-build-sysroot=/scratch/oe/sysroots/armv5te-oe-linux-gnueabi --enable-target-optspace --disable-libmudflap --disable-libgomp --disable-libssp --with-float=soft --program-prefix=arm-oe-linux-gnueabi- --enable-__cxa_atexit
Thread model: single
gcc version 4.5.1 20100530 (prerelease) (GCC)
Comment 1 Richard Biener 2010-06-02 19:14:14 UTC
Heh.  arm does have a bswapsi pattern though - why is that expanding to a call
at -Os?
Comment 2 Khem Raj 2010-06-02 22:52:43 UTC
(In reply to comment #1)
> Heh.  arm does have a bswapsi pattern though - why is that expanding to a call
> at -Os?
> 

IIUC this pattern only triggers for armv6+ architectures. The problem I see is on
a armv5te
Comment 3 Mikael Pettersson 2010-06-06 19:54:05 UTC
I've looked at this, and as far as I can tell it's caused by two separate improvements in 4.5 that are Ok individually but cause libgcc's __bswapsi2 to call itself when combined (and certain build options are selected).

1. The middle-end now recognises C idioms for bswap and represents them as calls to the builtin bswap.

2. The ARM backend now generates much better code for the builtin bswap, but for archs prior to ARMv6 when -Os is enabled the .md expander arranges to have a libcall emitted instead.

Consequently, then building for (say) ARMv5 with --enable-target-optspace:
a) the middle-end represents the C bswap idiom in libgcc2.c:__bswapsi2 () as a call to the builtin
b) the ARM backend (not knowing it's compiling libgcc) emits a libcall
c) hence __bswapsi2 () calls itself

The only way I see around this is to add a compiler option (-fin-libgcc say), set it when compiling libgcc routines (similar to -DIN_LIBGCC2), check for it in the ARM bswap expander, and if set ignore -Os in the arch < v6 case.
Comment 4 Richard Biener 2010-06-06 20:22:42 UTC
No, the arm backend should not claim to implement bswapsi when it in fact emits
a libcall.
Comment 5 Ramana Radhakrishnan 2010-06-22 00:51:47 UTC
Khem, 

Can you check if this fixes your problem ? Feel free to submit this to gcc-patches@ if you get around to testing this before me. 

Ramana


diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 628bd62..9dcceea 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -11296,7 +11296,7 @@
 (define_expand "bswapsi2"
   [(set (match_operand:SI 0 "s_register_operand" "=r")
        (bswap:SI (match_operand:SI 1 "s_register_operand" "r")))]
-"TARGET_EITHER"
+"TARGET_EITHER && (arm_arch6 || ( !arm_arch6 && !optimize_size))"
 "
   if (!arm_arch6)
     {
Comment 6 Jakub Jelinek 2010-06-22 05:50:58 UTC
Why not just:
TARGET_EITHER && (arm_arch6 || !optimize_size)
?  That !arm_arch6 && is completely redundant there.
Comment 7 Khem Raj 2010-06-22 15:42:34 UTC
(In reply to comment #6)
> Why not just:
> TARGET_EITHER && (arm_arch6 || !optimize_size)
> ?  That !arm_arch6 && is completely redundant there.
> 

Hi Ramana

the patch along with Jakub's suggestions works for me.

Thx

-Khem
Comment 8 Richard Biener 2010-06-24 21:38:25 UTC
If nobody tests and submits this patch the issue will be not fixed for 4.5.1.
Comment 9 Richard Biener 2010-07-31 09:29:51 UTC
GCC 4.5.1 is being released, adjusting target milestone.
Comment 10 Jakub Jelinek 2010-08-25 16:48:12 UTC
The patch has been approved, why hasn't been applied?
http://gcc.gnu.org/ml/gcc-patches/2010-07/msg00381.html
Comment 11 Ramana Radhakrishnan 2010-09-08 21:36:02 UTC
Subject: Bug 44392

Author: ramana
Date: Wed Sep  8 21:35:48 2010
New Revision: 164029

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=164029
Log:
2010-09-08  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	PR target/44392
	* config/arm/arm.md (bswapsi2): Handle condition correctly
	for armv6 and optimize_size.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/arm/arm.md

Comment 12 Jakub Jelinek 2010-11-29 18:24:14 UTC
Fixed on the trunk then.
Comment 13 Richard Biener 2010-12-16 13:03:43 UTC
GCC 4.5.2 is being released, adjusting target milestone.
Comment 14 Ramana Radhakrishnan 2011-01-25 07:18:09 UTC
Author: ramana
Date: Tue Jan 25 07:18:05 2011
New Revision: 169221

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=169221
Log:
2011-01-21  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	Backport from mainline.
	2010-09-08  Ramana Radhakrishnan  <ramana.radhakrishnan@arm.com>

	PR target/44392
	* config/arm/arm.md (bswapsi2): Handle condition correctly
	for armv6 and optimize_size.

Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/config/arm/arm.md
Comment 15 Ramana Radhakrishnan 2011-01-26 10:44:56 UTC
This is now fixed.