This is the mail archive of the gcc-bugs@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug target/67089] [4.8/4.9/5/6 Regression] Integer overflow checks not optimized on x86/x86_64


https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67089

--- Comment #1 from UroÅ Bizjak <ubizjak at gmail dot com> ---
(In reply to Mike from comment #0)
> Starting from 4.8.3, gcc does not optimize integer overflow and underflow
> checks inserting a redundant cmp instruction in both 32-bit and 64-bit
> modes. Having git-bisected the revisions, I found out r204088 (by pr58779)
> had added such behavior.

It was removed for a reason, as explained in the referred PR.

> ------------ gcc-4.8.3 -O1 ------------
> sub(unsigned int, unsigned int):
>     movl    %edi, %eax
>     subl    %esi, %eax
>     cmpl    %eax, %edi  /* absolutely redundant */
>     jnb .L4
>     subq    $8, %rsp
>     call    underflow()
> .L4:
>     rep ret
> ---------------------------------------

The sequence is actually:

(insn 7 4 8 2 (parallel [
            (set (reg/v:SI 87 [ r ])
                (minus:SI (reg/v:SI 89 [ a ])
                    (reg/v:SI 90 [ b ])))
            (clobber (reg:CC 17 flags))
        ]) pr67089.c:4 259 {*subsi_1}
     (expr_list:REG_DEAD (reg/v:SI 90 [ b ])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))
(insn 8 7 9 2 (set (reg:CC 17 flags)
        (compare:CC (reg/v:SI 89 [ a ])
            (reg/v:SI 87 [ r ]))) pr67089.c:5 7 {*cmpsi_1}
     (expr_list:REG_DEAD (reg/v:SI 89 [ a ])
        (nil)))
(jump_insn 9 8 10 2 (set (pc)
        (if_then_else (geu (reg:CC 17 flags)
                (const_int 0 [0]))
            (label_ref 13)
            (pc))) pr67089.c:5 609 {*jcc_1}
     (expr_list:REG_DEAD (reg:CC 17 flags)
        (int_list:REG_BR_PROB 9996 (nil)))
 -> 13)

and combine tries to simplify this sequence with:

Trying 7 -> 8:
Failed to match this instruction:
(parallel [
        (set (reg:CC 17 flags)
            (compare:CC (minus:SI (reg/v:SI 89 [ a ])
                    (reg/v:SI 90 [ b ]))
                (reg/v:SI 89 [ a ])))
        (set (reg/v:SI 87 [ r ])
            (minus:SI (reg/v:SI 89 [ a ])
                (reg/v:SI 90 [ b ])))
    ])

and further to:

Trying 7, 8 -> 9:
Failed to match this instruction:
(parallel [
        (set (pc)
            (if_then_else (leu (minus:SI (reg/v:SI 89 [ a ])
                        (reg/v:SI 90 [ b ]))
                    (reg/v:SI 89 [ a ]))
                (label_ref 13)
                (pc)))
        (set (reg/v:SI 87 [ r ])
            (minus:SI (reg/v:SI 89 [ a ])
                (reg/v:SI 90 [ b ])))
    ])

LEU condition was created from GEU as the operands in (insn 8) were swapped. As
can be seen from config/i386.c, ix86_cc_mode, LEU can't be implemented using
only Carry flag:

    case GTU:                   /* CF=0 & ZF=0 */
    case LEU:                   /* CF=1 | ZF=1 */
      return CCmode;

The removed code compensated this with *reversion* of the flag check, e.g. the
same condition was emitted for LEU with CCCmode as for GEU (and in a similar
for GTU in CCCmode vs. LTU, as was the case in PR58779).

We shouldn't do this, and it reflected in PR58779.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]