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 rtl-optimization/82628] [8 Regression] wrong code at -Os on x86_64-linux-gnu in the 32-bit mode


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

--- Comment #15 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Jakub Jelinek from comment #9)
> So, for the -1UL <= 1UL comparison, the cmpl instruction sets the CF, and
> then
> sbbl subtracts -1U (2nd operand) + 1U (CF) from 0U (1st operand/destination)
> and sets CF flag, but the RTL representation of this, i.e. comparison of
> (2nd operand + CF) and (1st operand), i.e. 0 and 0, doesn't set the CF.
> 
> From the description of SBBL, it is unclear if it sets CF according to
> op1 - (SImode) (op2 + CF) or (DImode) op1 - ((DImode) op2 + CF), but from
> the above it looks like in order to do this we rely on the latter, i.e. it
> is actually 0 - (0xffffffffULL + 1) == 0 - 0x100000000ULL = 0 with CF.
> 
> So, do we need to represent it differently in the instruction, as comparison
> in twice as wide mode?  I think we'd then need different patterns for signed
> and unsigned comparisons, one would sign-extend, the other zero-extend, and
> then perhaps just use CCOmode or CCZmode.

I was thinking to introduce cmp_doubleword and split it after reload. This is
the approach all other _doubleword patterns take, and hides from combine the
fact that these instructions are not 100% correctly described. IMO, this would
be the safest approach, and would also keep the true functionality of the
instruction up until the reload.

I though that since all registers die in the insn, it would be OK to split it
at the expand time. Apparently, it was not ;)

> Do we need to fix also subborrow<mode>?  What about the addcarry<mode>?

This is different, but related issue. IMO, we can make current patterns [the
ones with (plus (ltu (reg:CC 17 flags)(const_int 0 [0]))] available only after
reload (with a comment that they are not 100% correct) - thus hide them from
combine, and implement subborrow and addcarry with an unspec. These two are
generated from bultins anyway, so they are not expected to be combined.

If you agree, I'd propose to fix this PR with the introduction of
cmp_doubleword and open new PR to deal with subborrow and addcarry unspecs.

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