Bug 11969 - -foptimize-sibling-calls from -O2 creates bad code
Summary: -foptimize-sibling-calls from -O2 creates bad code
Alias: None
Product: gcc
Classification: Unclassified
Component: rtl-optimization (show other bugs)
Version: 3.2.2
: P2 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
Keywords: wrong-code
Depends on:
Reported: 2003-08-18 16:23 UTC by George R.
Modified: 2005-07-23 22:49 UTC (History)
2 users (show)

See Also:
Known to work:
Known to fail:
Last reconfirmed:


Note You need to log in before you can comment on or make changes to this bug.
Description George R. 2003-08-18 16:23:10 UTC
The ICU (International Components for Unicode) project keeps on getting bug 
reports about how ICU won't build with the -O2 or -Os optimization flags. After 
many hours of debugging, I've been able to narrow the problem down.

I have narrowed it down to the -foptimize-sibling-calls option that is included 
with the -O2 and -Os flags. It causes one of our tools to generate an internal 
program error. It seems to affect only one file in ICU 
(icu/source/i18n/ucol_wgt.c). You can see a copy of the file here: 

We know it fails with Red Hat Linux 8 and 9 on x86. Other people on Mac OS and 
Solaris have also reported the same problem. At least gcc 3.2, 3.2.2 and 3.3 
fail to properly optimize the code.

To get around this problem we use -O3. I think it includes the -Winline option, 
which seems to be the work around for now. If I declare all of the static 
functions as inline in ucol_wgt.c, the build problem will also disappear.

I'm not exactly sure what the exact problem is in the generated assembly code, 
but the -foptimize-sibling-calls option is the source of the problem. I'm just 
worried that this option causes something else to fail too, and we haven't run 
across is yet.

I know you guys don't like to download projects, but you can download the 
source code for testing from here: 

ICU 2.6 is known to have this problem, but other previous version are known to 
have it too, like ICU 2.2 and ICU 2.4.

BTW thanks for listing the optimization flags used in the gcc 3.3 
documentation. It helped a lot.
Comment 1 Andrew Pinski 2003-08-18 16:36:16 UTC
Is there a simple example which shows this problem like the the test in the source or is there do 
we have to find something else to test it with?
Comment 2 George R. 2003-08-18 17:28:15 UTC
I haven't been able to find a simplier example that reproduces this bug yet.

Though I have narrowed the problem down a bit further. If I just declare the 
function incWeight() as the following, the build error for ja.txt goes away. 
Maybe the other functions don't need to be inlined.

static U_INLINE uint32_t
incWeight(uint32_t weight, int32_t length, uint32_t maxByte) {

I just know that this code initializes several data structures incorrectly, and 
it causes our genrb build tool to fail. Specifically it fails during the build 
of ICU when this is run.

(Information from our bug reports)

On Solaris 
IBRARY_PATH  ../tools/genrb/genrb -k -q -p icudt24l -s ../data/locales -d
../data/out/build ja.txt
../data/locales/ja.txt:15: parse error. Stopped parsing with
couldn't parse the file ja.txt. Error:U_INVALID_FORMAT_ERROR
make[1]: *** [../data/out/build/icudt24l_ja.res] Error 3
make[1]: Leaving directory `/export/home/sys/test/icu/source/data'
make: *** [all-recursive] Error 2

On Linux
 ../tools/genrb/genrb -k -q -p icudt26l -s ../data/locales -d ../data/out/build
../data/locales/ja.txt:15: parse error. Stopped parsing with
couldn't parse the file ja.txt. Error:U_INVALID_FORMAT_ERROR
make[1]: *** [../data/out/build/icudt26l_ja.res] Fehler 3
make[1]: Leaving directory `/workspace/software/libraries/icu/source/data'
Comment 3 Andrew Pinski 2003-08-18 18:10:27 UTC
Feedback recieved so marking as invalid to mark ...
Comment 4 Andrew Pinski 2003-08-18 18:10:51 UTC
Comment 5 Steven Bosscher 2003-09-06 15:52:36 UTC
Here are all the replacements that replace_call_placeholder
does with "-O2 -fno-unit-at-a-time":

getWeightByte -> getWeightTrail
incWeight -> setWeightByte
incWeight -> getWeightByte
incWeight -> setWeightByte
lengthenRange -> setWeightTrail
lengthenRange -> setWeightTrail
getWeightRanges -> lengthOfWeight
getWeightRanges -> lengthOfWeight
getWeightRanges -> truncateWeight
getWeightRanges -> memset
getWeightRanges -> memset
getWeightRanges -> getWeightTrail
getWeightRanges -> getWeightTrail
getWeightRanges -> setWeightTrail
getWeightRanges -> truncateWeight
getWeightRanges -> incWeightTrail
getWeightRanges -> getWeightTrail
getWeightRanges -> setWeightTrail
getWeightRanges -> decWeightTrail
getWeightRanges -> truncateWeight
getWeightRanges -> decWeightTrail
getWeightRanges -> incWeight
getWeightRanges -> getWeightTrail
getWeightRanges -> getWeightTrail
getWeightRanges -> getWeightByte
getWeightRanges -> getWeightByte
ucol_allocWeights_2_6 -> getWeightRanges
ucol_allocWeights_2_6 -> lengthenRange
ucol_allocWeights_2_6 -> getWeightByte
ucol_allocWeights_2_6 -> setWeightByte
ucol_allocWeights_2_6 -> incWeight
ucol_allocWeights_2_6 -> truncateWeight
ucol_allocWeights_2_6 -> incWeight
ucol_allocWeights_2_6 -> lengthenRange
ucol_allocWeights_2_6 -> lengthenRange
ucol_allocWeights_2_6 -> qsort

So there are only two sibcalls, and all the other "potentials" are replaced with
normal calls.

If this problem is real, then the sibcall incWeight -> setWeightByte may be the
cause of it.  But the RTL for this looks quite OK AFAICT... :-\
Comment 6 Steven Bosscher 2003-09-13 09:15:28 UTC
Maybe this patch fixes it for you:

But of course there's no way for us to tell without a test case.
Comment 7 Dara Hazeghi 2004-01-17 23:39:53 UTC
No testcase, and no feedback on the patch Steven mentioned. If you get a chance to test this again 
(preferably with gcc 3.3.2 or a snapshot), we can reopen this.