[PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Uros Bizjak]

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

On Tue, Feb 7, 2012 at 1:59 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes.

Actually, CCZ mode is not compatible with CCNO mode, since the later
only declares that overflow flag is not set. CCGOC and CCGO declare
garbage in overflow (and carry in case of CCGOC) flag, so implicitly
declare that CCZ flag is valid. Following this reasoning, CCZ mode
should be compatible with CCGOC and CCGO modes.

2012-02-07  Uros Bizjak  <ubizjak@gmail.com>

       * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode
       compatible with CCGOCmode and CCGCmode.

Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu.

Uros.

[-- Attachment #2: p.diff.txt --]
[-- Type: text/plain, Size: 517 bytes --]

Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 183968)
+++ config/i386/i386.c	(working copy)
@@ -17778,6 +17778,11 @@ ix86_cc_modes_compatible (enum machine_mode m1, en
       || (m1 == CCGOCmode && m2 == CCGCmode))
     return CCGCmode;
 
+  if (m1 == CCZmode && (m2 == CCGCmode || m2 == CCGOCmode))
+    return m2;
+  else if (m2 == CCZmode && (m1 == CCGCmode || m1 == CCGOCmode))
+    return m1;
+
   switch (m1)
     {
     default:

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Richard Henderson]

On 02/07/2012 08:37 AM, Uros Bizjak wrote:
> On Tue, Feb 7, 2012 at 1:59 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
> 
>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes.
> 
> Actually, CCZ mode is not compatible with CCNO mode, since the later
> only declares that overflow flag is not set. CCGOC and CCGO declare
> garbage in overflow (and carry in case of CCGOC) flag, so implicitly
> declare that CCZ flag is valid. Following this reasoning, CCZ mode
> should be compatible with CCGOC and CCGO modes.

We should probably fix this confusion by renaming the modes so that
they're all positive:

  CCNO		-> CCCSZ
  CCGC		-> CCOSZ
  CCGOC		-> CCSZ


r~

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Uros Bizjak]

On Tue, Feb 7, 2012 at 5:57 PM, Richard Henderson <rth@redhat.com> wrote:

>>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes.
>>
>> Actually, CCZ mode is not compatible with CCNO mode, since the later
>> only declares that overflow flag is not set. CCGOC and CCGO declare
>> garbage in overflow (and carry in case of CCGOC) flag, so implicitly
>> declare that CCZ flag is valid. Following this reasoning, CCZ mode
>> should be compatible with CCGOC and CCGO modes.
>
> We should probably fix this confusion by renaming the modes so that
> they're all positive:
>
>  CCNO          -> CCCSZ
>  CCGC          -> CCOSZ
>  CCGOC         -> CCSZ

No, no. Once you get the logic, it actually makes sense to name them
that way. Regarding your proposed change, CCNO does not say that CSZ
are all valid, it says that OF = 0, so the first line is wrong.

Uros.

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Uros Bizjak]

On Tue, Feb 7, 2012 at 5:37 PM, Uros Bizjak <ubizjak@gmail.com> wrote:

>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes.
>
> Actually, CCZ mode is not compatible with CCNO mode, since the later
> only declares that overflow flag is not set. CCGOC and CCGO declare
> garbage in overflow (and carry in case of CCGOC) flag, so implicitly
> declare that CCZ flag is valid. Following this reasoning, CCZ mode
> should be compatible with CCGOC and CCGO modes.
>
> 2012-02-07  Uros Bizjak  <ubizjak@gmail.com>
>
>       * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode
>       compatible with CCGOCmode and CCGCmode.
>
> Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu.

... where it uncovers another problem how RTL optimizer handles
compatible compares!

With attached patch, the example from pr28685

--cut here--
int test(int a, int b)
{
  int lt = a < b;
  int eq = a == b;
  if (lt)
    return 1;
  return eq;
}
--cut here--

triggers compare elimination in CSE2 pass. We enter CSE2 pass with:

(insn 8 5 9 2 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg/v:SI 62 [ a ])
            (reg/v:SI 63 [ b ]))) pr28685.c:7 6 {*cmpsi_1}
     (nil))

(jump_insn 9 8 10 2 (set (pc)
        (if_then_else (lt (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (label_ref:DI 15)
            (pc))) pr28685.c:7 599 {*jcc_1}
     (expr_list:REG_DEAD (reg:CCGC 17 flags)
        (expr_list:REG_BR_PROB (const_int 3900 [0xf3c])
            (nil)))
 -> 15)

[...]

(note 10 9 11 3 [bb 3] NOTE_INSN_BASIC_BLOCK)

(insn 11 10 12 3 (set (reg:CCZ 17 flags)
        (compare:CCZ (reg/v:SI 62 [ a ])
            (reg/v:SI 63 [ b ]))) pr28685.c:6 6 {*cmpsi_1}
     (expr_list:REG_DEAD (reg/v:SI 63 [ b ])
        (expr_list:REG_DEAD (reg/v:SI 62 [ a ])
            (nil))))

(insn 12 11 13 3 (set (reg:QI 65)
        (eq:QI (reg:CCZ 17 flags)
            (const_int 0 [0]))) pr28685.c:6 595 {*setcc_qi}
     (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (nil)))

After CSE2 pass, we have:

(insn 8 5 9 2 (set (reg:CCGC 17 flags)
        (compare:CCGC (reg/v:SI 62 [ a ])
            (reg/v:SI 63 [ b ]))) pr28685.c:7 6 {*cmpsi_1}
     (nil))

(jump_insn 9 8 10 2 (set (pc)
        (if_then_else (lt (reg:CCGC 17 flags)
                (const_int 0 [0]))
            (label_ref:DI 15)
            (pc))) pr28685.c:7 599 {*jcc_1}
     (expr_list:REG_DEAD (reg:CCGC 17 flags)
        (expr_list:REG_BR_PROB (const_int 3900 [0xf3c])
            (nil)))
 -> 15)

[...]

(note 10 9 12 3 [bb 3] NOTE_INSN_BASIC_BLOCK)

(insn 12 10 13 3 (set (reg:QI 65)
        (eq:QI (reg:CCGC 17 flags)
            (const_int 0 [0]))) pr28685.c:6 595 {*setcc_qi}
     (expr_list:REG_DEAD (reg:CCGC 17 flags)
        (nil)))

CSE2 pass eliminated (insn 11), which is OK since CCZ is compatible
with CCGC, so (CCGC, CCZ)->CCGC. However, the pass also changed the
mode of flags register in (insn 12). I don't think this is correct,
the user should not be changed at all.

Uros.

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Uros Bizjak]

On Sat, Feb 11, 2012 at 9:41 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

>>> Attached patch declares CCZmode compatible with CCGOC, CCGO and CCNO modes.
>>
>> Actually, CCZ mode is not compatible with CCNO mode, since the later
>> only declares that overflow flag is not set. CCGOC and CCGO declare
>> garbage in overflow (and carry in case of CCGOC) flag, so implicitly
>> declare that CCZ flag is valid. Following this reasoning, CCZ mode
>> should be compatible with CCGOC and CCGO modes.
>>
>> 2012-02-07  Uros Bizjak  <ubizjak@gmail.com>
>>
>>       * config/i386/i386.c (ix86_cc_modes_compatible): Declare CCZmode
>>       compatible with CCGOCmode and CCGCmode.
>>
>> Attached patch was bootstrapped and regression tested on x86_64-pc-linux-gnu.
>
> ... where it uncovers another problem how RTL optimizer handles
> compatible compares!

> CSE2 pass eliminated (insn 11), which is OK since CCZ is compatible
> with CCGC, so (CCGC, CCZ)->CCGC. However, the pass also changed the
> mode of flags register in (insn 12). I don't think this is correct,
> the user should not be changed at all.

This is also where my proposed patch [1] for post-reload compare
elimination differs from CSE2 pass. The proposed revision doesn't
update the mode of flags reg of users to calculated compatible mode.
FWIW, the mode of flags in users doesn't matter at all on x86, but
which way is correct?

[1] http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00466.html

Uros.

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Richard Henderson]

On 02/11/2012 12:56 AM, Uros Bizjak wrote:
> FWIW, the mode of flags in users doesn't matter at all on x86, but
> which way is correct?

As far as I know, it doesn't matter anywhere.  We don't even bother to have perfect harmony between  integer modes in hard registers -- think about what happens when we drop all the subregs on the floor post-reload.  Yes, it's probably an error if we don't have compatible modes between def and use, but nothing is going to check for that.


r~

Re: [PATCH 4.8 v2, i386]: Make CCZ mode compatible with CCGOC and CCGO modes

[Uros Bizjak]

On Tue, Feb 14, 2012 at 12:00 AM, Richard Henderson <rth@redhat.com> wrote:
> On 02/11/2012 12:56 AM, Uros Bizjak wrote:
>> FWIW, the mode of flags in users doesn't matter at all on x86, but
>> which way is correct?
>
> As far as I know, it doesn't matter anywhere.  We don't even bother to have perfect harmony between  integer modes in hard registers -- think about what happens when we drop all the subregs on the floor post-reload.  Yes, it's probably an error if we don't have compatible modes between def and use, but nothing is going to check for that.

cse.c says some relaxing words related to this issue:

  /* If the following assertion was triggered, there is most probably
     something wrong with the cc_modes_compatible back end function.
     CC modes only can be considered compatible if the insn - with the mode
     replaced by any of the compatible modes - can still be recognized.  */

It looks to me that correct definition of cc_modes_compatible
guarantees that insn is still valid, no matter if the mode of flags
remains in the "wrong mode".

In any case, I will add the comment to avoid confusion.

Thanks,
Uros.