etienne@cygne:~$ gcc -v Using built-in specs. Target: i486-linux-gnu Configured with: ../src/configure -v --enable-languages=c,c++,fortran,objc,obj-c++,treelang --prefix=/usr --enable-shared --with-system-zlib --libexecdir=/usr/lib --without-included-gettext --enable-threads=posix --enable-nls --program-suffix=-4.1 --enable-__cxa_atexit --enable-clocale=gnu --enable-libstdcxx-debug -- enable-mpfr --with-tune=i686 --enable-checking=release i486-linux-gnu Thread model: posix gcc version 4.1.2 20060729 (prerelease) (Debian 4.1.1-10) etienne@cygne:~$ cat tmp1.c int fct1(void); int fct2(void); int fct (unsigned nb) { if ((nb >> 5) != 0) return fct1(); else return fct2(); } etienne@cygne:~$ gcc -O3 -fomit-frame-pointer -S tmp1.c -o tmp1.s etienne@cygne:~$ cat tmp1.s .file "tmp1.c" .text .p2align 4,,15 .globl fct .type fct, @function fct: movl 4(%esp), %eax shrl $5, %eax testl %eax, %eax je .L2 jmp fct1 .p2align 4,,7 .L2: jmp fct2 .size fct, .-fct .ident "GCC: (GNU) 4.1.2 20060729 (prerelease) (Debian 4.1.1-10)" .section .note.GNU-stack,"",@progbits etienne@cygne:~$ The assembly instruction "testl %eax, %eax" is not needed considering the Intel documentation of "SAL/SAR/SHL/SHR", "Flags Affected": The SF, ZF, and PF flags are set according to the result.
Confirmed, a regression from 2.95.3 which almost means the new ia32 back-end caused this.
It is entirely coincident. For some processors, it is an optimization to avoid partial flag register stall. When it is fixed, it should be reenabled with a new flag, something like TARGET_PARTIAL_FLAG_REG_STALL.
This specific case can probably be solved at the tree level by changing the test: (nb >> 5) != 0 to nb > 32
(In reply to comment #2) > It is entirely coincident. For some processors, it is an optimization to avoid > partial flag register stall. When it is fixed, it should be reenabled with a > new flag, something like TARGET_PARTIAL_FLAG_REG_STALL. There is TARGET_USE_INCDEC flag that already implements your suggestion. From predicates.md: /* On Pentium4, the inc and dec operations causes extra dependency on flag registers, since carry flag is not set. */ if (!TARGET_USE_INCDEC && !optimize_size) If used elsewhere, this flag should perhaps be renamed to proposed TARGET_PARTIAL_FLAG_REG_STALL.
The problem here is following: We already have the patterns, that would satisfy combined instruction (*lshrsi3_cmp) in above testcase. However, combiner rejects combined instruction because the register that holds shifted result is unused! The problematic part is in combine.c, around line 2236 (please read the comment, which describes exactly the situation we have here). This part of code is activated only when the register that holds the result of arith operation is keept alive. This is quite strange - even if the result is unused, resulting code will be still smaller as we avoid extra CC setting instruction. The patch bellow (currently under testing, but so far OK) forces generation of combined instruction even if the arithmetic result is unused. Index: combine.c =================================================================== --- combine.c (revision 116691) +++ combine.c (working copy) @@ -2244,7 +2244,7 @@ needed, and make the PARALLEL by just replacing I2DEST in I3SRC with I2SRC. Later we will make the PARALLEL that contains I2. */ - if (i1 == 0 && added_sets_2 && GET_CODE (PATTERN (i3)) == SET + if (i1 == 0 && GET_CODE (PATTERN (i3)) == SET && GET_CODE (SET_SRC (PATTERN (i3))) == COMPARE && XEXP (SET_SRC (PATTERN (i3)), 1) == const0_rtx && rtx_equal_p (XEXP (SET_SRC (PATTERN (i3)), 0), i2dest)) @@ -2254,6 +2254,13 @@ enum machine_mode compare_mode; #endif + /* To force generation of the combined comparison and arithmetic + operation PARALLEL, pretend that the set in I2 is to be used, + even if it is dead after I2. This results in better generated + code, as only CC setting arithmetic instruction will be + emitted in conditionals. */ + added_sets_2 = 1; + newpat = PATTERN (i3); SUBST (XEXP (SET_SRC (newpat), 0), i2src); Compiling testcase with this patch results in following code: fct: movl 4(%esp), %eax shrl $5, %eax je .L2 jmp fct1 .p2align 4,,7 .L2: jmp fct2
Patch at http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00137.html BTW: This patch eliminates 869 "test" instructions in povray-3.6.1 compile. (And my test raytraced pictures are still correct.)
Hm, proposed patch now generates worse code for following test: extern int fnc1(void); extern int fnc2(void); int test(int x) { if (x & 0x02) return fnc1(); else if (x & 0x01) return fnc2(); else return 0; } It generates: test: movl 4(%esp), %edx movl %edx, %eax andl $2, %eax jne .L10 andl $1, %edx jne .L11 xorl %eax, %eax ret .p2align 4,,7 .L11: .p2align 4,,8 jmp fnc2 .p2align 4,,7 .L10: .p2align 4,,7 jmp fnc1 due to marking %eax live in first comparison, "and" is used instead of "test", and a regmove is emitted before comparison. Ideally gcc should generate: test: movl 4(%esp), %eax testl $2, %eax jne .L6 andl $1, %eax jne .L7 xorl %eax, %eax ret .p2align 2,,3 .L7: jmp fnc2 .p2align 2,,3 .L6: jmp fnc1
TARGET_PARTIAL_FLAG_REG_STALL and TARGET_USE_INCDEC are totally different. TARGET_USE_INCDEC favors inc/dec over add/sub while TARGET_PARTIAL_FLAG_REG_STALL adds test after shift.
Patch at http://gcc.gnu.org/ml/gcc-patches/2006-09/msg00162.html implements missing i386.md RTL patterns. This is i386 target-specific fix for this bug. The patch was bootstrapped on i686-pc-linux-gnu and x86_64-pc-linux-gnu, regtested for c,c++ and fortran.
The proposed patch will slow down Core and Core 2 by 70-100% in some testcases due to partial flag register stall. I have a followup patch to implement TARGET_PARTIAL_FLAG_REG_STALL.
Subject: Bug 28946 Author: uros Date: Thu Sep 7 17:45:48 2006 New Revision: 116756 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116756 Log: PR target/28946 * config/i386/i386.md ("*ashldi3_cconly_rex64", "*ashlsi3_cconly", "*ashlhi3_cconly", "*ashlqi3_cconly", "*ashrdi3_one_bit_cconly_rex64", "*ashrdi3_cconly_rex64", "*ashrsi3_one_bit_cconly", "*ashrsi3_cconly", "*ashrhi3_one_bit_cconly", "*ashrhi3_cconly", "*ashrqi3_one_bit_cconly", "*ashrqi3_cconly", "*lshrdi3_cconly_one_bit_rex64", "*lshrdi3_cconly_rex64", "*lshrsi3_one_bit_cconly", "*lshrsi3_cconly", "*lshrhi3_one_bit_cconly", "*lshrhi3_cconly", "*lshrqi2_one_bit_cconly", "*lshrqi2_cconly": New patterns to implement only CC setting effects of shift instructions. testsuite/ChangeLog: PR target/28946 * gcc.target/i386/pr28946.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr28946.c Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md trunk/gcc/testsuite/ChangeLog
Subject: Bug 28946 Author: uros Date: Fri Sep 15 17:42:40 2006 New Revision: 116979 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=116979 Log: PR target/28946 * config/i386/i386.md ("*ashldi3_cconly_rex64", "*ashlsi3_cconly", "*ashlhi3_cconly", "*ashlqi3_cconly", "*ashrdi3_one_bit_cconly_rex64", "*ashrdi3_cconly_rex64", "*ashrsi3_one_bit_cconly", "*ashrsi3_cconly", "*ashrhi3_one_bit_cconly", "*ashrhi3_cconly", "*ashrqi3_one_bit_cconly", "*ashrqi3_cconly", "*lshrdi3_cconly_one_bit_rex64", "*lshrdi3_cconly_rex64", "*lshrsi3_one_bit_cconly", "*lshrsi3_cconly", "*lshrhi3_one_bit_cconly", "*lshrhi3_cconly", "*lshrqi2_one_bit_cconly", "*lshrqi2_cconly": New patterns to implement only CC setting effects of shift instructions. testsuite/ChangeLog: PR target/28946 * gcc.target/i386/pr28946.c: New test. Added: branches/gcc-4_1-branch/gcc/testsuite/gcc.target/i386/pr28946.c Modified: branches/gcc-4_1-branch/gcc/ChangeLog branches/gcc-4_1-branch/gcc/config/i386/i386.md branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
Subject: Bug 28946 Author: uros Date: Mon Sep 18 10:14:53 2006 New Revision: 117022 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=117022 Log: PR target/28946 * config/i386/i386.md ("*ashldi3_cconly_rex64", "*ashlsi3_cconly", "*ashlhi3_cconly", "*ashlqi3_cconly", "*ashrdi3_one_bit_cconly_rex64", "*ashrdi3_cconly_rex64", "*ashrsi3_one_bit_cconly", "*ashrsi3_cconly", "*ashrhi3_one_bit_cconly", "*ashrhi3_cconly", "*ashrqi3_one_bit_cconly", "*ashrqi3_cconly", "*lshrdi3_cconly_one_bit_rex64", "*lshrdi3_cconly_rex64", "*lshrsi3_one_bit_cconly", "*lshrsi3_cconly", "*lshrhi3_one_bit_cconly", "*lshrhi3_cconly", "*lshrqi2_one_bit_cconly", "*lshrqi2_cconly": New patterns to implement only CC setting effects of shift instructions. testsuite/ChangeLog: PR target/28946 * gcc.target/i386/pr28946.c: New test. Added: branches/gcc-4_0-branch/gcc/testsuite/gcc.target/i386/pr28946.c Modified: branches/gcc-4_0-branch/gcc/ChangeLog branches/gcc-4_0-branch/gcc/config/i386/i386.md branches/gcc-4_0-branch/gcc/testsuite/ChangeLog
Fixed everywhere.