[c] all plattforms: support using a CC_REG instead cc0_rtx

Hi there,

I suggest this patch to allow architectures do substitute cc0_rtx with a 
generated cc register.

Why? If you are using a cc register plus your architecture as many 
instructions which may clobber that cc register, some passes (e.g. gcse) 
will reorder the insns. This can lead to the situation that an insn is 
moved between a compare and it' consuming jump insn. Which yields 
invalid code. (Note that at these stages clobbers are not yet tracked as 
CLOBBER insns).

To get around this behaviour you have to fake HAVE_CC0, e.g. by adding 
some dummy code to your md file:

    (define_peephole2 [(set (match_operand 0 "" "") (cc0))] "" 
[(const_int 0)] "")

plus some empty macros. All of this can be handled by the arch 
implementation, but not the check for the cc0 inside of

   int sets_cc0_p(const_rtx x)


Changelog

     gcc/jump.c: use rtx_equal do determine the identity of cc0_rtx

======

Hi!

On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
> I suggest this patch to allow architectures do substitute cc0_rtx with a 
> generated cc register.
> 
> Why? If you are using a cc register plus your architecture as many 
> instructions which may clobber that cc register, some passes (e.g. gcse) 
> will reorder the insns. This can lead to the situation that an insn is 
> moved between a compare and it' consuming jump insn. Which yields 
> invalid code. (Note that at these stages clobbers are not yet tracked as 
> CLOBBER insns).

Then that port has a bug.  In the m68k port, there are no separate compare
and jump insns ever, but for most ports those won't yet exist during gcse.

This is not unique to cc0 conversions: every port has a similar problem
with all FIXED_REGISTERS.

> --- a/gcc/jump.c
> +++ b/gcc/jump.c
> @@ -1028,7 +1028,7 @@ sets_cc0_p (const_rtx x)
>    if (INSN_P (x))
>      x = PATTERN (x);
> 
> -  if (GET_CODE (x) == SET && SET_DEST (x) == cc0_rtx)
> +  if (GET_CODE (x) == SET && rtx_equal_p(SET_DEST (x), cc0_rtx))
>      return 1;

@findex cc0_rtx
There is only one expression object of code @code{cc0}; it is the
value of the variable @code{cc0_rtx}.  Any attempt to create an
expression of code @code{cc0} will return @code{cc0_rtx}.

There is a lot of code that depends on this property, you cannot break
it without fixing everything.


Segher

Am 2019-12-13 18:58, schrieb Segher Boessenkool:
> Hi!
> 
> On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
>> I suggest this patch to allow architectures do substitute cc0_rtx with 
>> a
>> generated cc register.
>> 
>> Why? If you are using a cc register plus your architecture as many
>> instructions which may clobber that cc register, some passes (e.g. 
>> gcse)
>> will reorder the insns. This can lead to the situation that an insn is
>> moved between a compare and it' consuming jump insn. Which yields
>> invalid code. (Note that at these stages clobbers are not yet tracked 
>> as
>> CLOBBER insns).
> 
> Then that port has a bug.  In the m68k port, there are no separate 
> compare
> and jump insns ever, but for most ports those won't yet exist during 
> gcse.

it looks like t2o insn for the m68k

(insn 115 114 116 5 (set (cc0)
         (compare (subreg:HI (reg:SI 272) 2)
             (reg:HI 273))) 
/tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216 
17 {*m68k.md:559}
      (nil))
(jump_insn 116 115 117 5 (set (pc)
         (if_then_else (ne (cc0)
                 (const_int 0 [0]))
             (label_ref 99)
             (pc))) 
/tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216 
411 {bne}
      (int_list:REG_BR_PROB 4 (nil))
  -> 99)


and the i386 uses too two insns:

(insn 17 16 18 4 (set (reg:CCNO 17 flags)
         (compare:CCNO (reg/v:SI 96 [ <retval> ])
             (const_int 0 [0]))) "x.c":2 3 {*cmpsi_ccno_1}
      (nil))
(jump_insn 18 17 19 4 (set (pc)
         (if_then_else (le (reg:CCNO 17 flags)
                 (const_int 0 [0]))
             (label_ref:DI 28)
             (pc))) "x.c":2 627 {*jcc_1}
      (int_list:REG_BR_PROB 1500 (nil))


Now consider an insn gets moved in between modifiyng that status 
register...


> This is not unique to cc0 conversions: every port has a similar problem
> with all FIXED_REGISTERS.

It's not related to fixed registers. It's unique to CC registers since 
these are on some plattforms modified by side effects. So after split2 
it's modelled using CLOBBERs


>> --- a/gcc/jump.c
>> +++ b/gcc/jump.c
>> @@ -1028,7 +1028,7 @@ sets_cc0_p (const_rtx x)
>>    if (INSN_P (x))
>>      x = PATTERN (x);
>> 
>> -  if (GET_CODE (x) == SET && SET_DEST (x) == cc0_rtx)
>> +  if (GET_CODE (x) == SET && rtx_equal_p(SET_DEST (x), cc0_rtx))
>>      return 1;
> 
> @findex cc0_rtx
> There is only one expression object of code @code{cc0}; it is the
> value of the variable @code{cc0_rtx}.  Any attempt to create an
> expression of code @code{cc0} will return @code{cc0_rtx}.
> 
> There is a lot of code that depends on this property, you cannot break
> it without fixing everything.
> 
> 
> Segher

There is no need to change the definition or modify any piece elsewhere. 
And the modified comparison will still work for cc0.


Stefan

On Fri, Dec 13, 2019 at 08:55:15PM +0100, Stefan Franke wrote:
> Am 2019-12-13 18:58, schrieb Segher Boessenkool:
> >On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
> >>Why? If you are using a cc register plus your architecture as many
> >>instructions which may clobber that cc register, some passes (e.g. 
> >>gcse)
> >>will reorder the insns. This can lead to the situation that an insn is
> >>moved between a compare and it' consuming jump insn. Which yields
> >>invalid code. (Note that at these stages clobbers are not yet tracked 
> >>as
> >>CLOBBER insns).
> >
> >Then that port has a bug.  In the m68k port, there are no separate 
> >compare
> >and jump insns ever, but for most ports those won't yet exist during 
> >gcse.
> 
> it looks like t2o insn for the m68k
> 
> (insn 115 114 116 5 (set (cc0)
>         (compare (subreg:HI (reg:SI 272) 2)
>             (reg:HI 273))) 
> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216 
> 17 {*m68k.md:559}
>      (nil))
> (jump_insn 116 115 117 5 (set (pc)
>         (if_then_else (ne (cc0)
>                 (const_int 0 [0]))
>             (label_ref 99)
>             (pc))) 
> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216 
> 411 {bne}
>      (int_list:REG_BR_PROB 4 (nil))
>  -> 99)

This is an older compiler.  m68k no longer uses cc0 (except it is still
mentioned in two comments (well, commented-out code)).

> >This is not unique to cc0 conversions: every port has a similar problem
> >with all FIXED_REGISTERS.
> 
> It's not related to fixed registers.

No, it is exactly the same situation.  You cannot introduce uses of such
a register if it might already exist in the insn stream somewhere, not
without checking first, and you better have a backup plan too.

> It's unique to CC registers since 
> these are on some plattforms modified by side effects. So after split2 
> it's modelled using CLOBBERs

There are no such implicit side effects if you have gotten rid of cc0.
That is the *point* of removing cc0.

> >@findex cc0_rtx
> >There is only one expression object of code @code{cc0}; it is the
> >value of the variable @code{cc0_rtx}.  Any attempt to create an
> >expression of code @code{cc0} will return @code{cc0_rtx}.
> >
> >There is a lot of code that depends on this property, you cannot break
> >it without fixing everything.
> 
> There is no need to change the definition or modify any piece elsewhere. 
> And the modified comparison will still work for cc0.

Then you do not need your patch.  You can compare cc0_rtx by identity.


Segher

Am 2019-12-13 21:59, schrieb Segher Boessenkool:
> On Fri, Dec 13, 2019 at 08:55:15PM +0100, Stefan Franke wrote:
>> Am 2019-12-13 18:58, schrieb Segher Boessenkool:
>> >On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
>> >>Why? If you are using a cc register plus your architecture as many
>> >>instructions which may clobber that cc register, some passes (e.g.
>> >>gcse)
>> >>will reorder the insns. This can lead to the situation that an insn is
>> >>moved between a compare and it' consuming jump insn. Which yields
>> >>invalid code. (Note that at these stages clobbers are not yet tracked
>> >>as
>> >>CLOBBER insns).
>> >
>> >Then that port has a bug.  In the m68k port, there are no separate
>> >compare
>> >and jump insns ever, but for most ports those won't yet exist during
>> >gcse.
>> 
>> it looks like t2o insn for the m68k
>> 
>> (insn 115 114 116 5 (set (cc0)
>>         (compare (subreg:HI (reg:SI 272) 2)
>>             (reg:HI 273)))
>> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
>> 17 {*m68k.md:559}
>>      (nil))
>> (jump_insn 116 115 117 5 (set (pc)
>>         (if_then_else (ne (cc0)
>>                 (const_int 0 [0]))
>>             (label_ref 99)
>>             (pc)))
>> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
>> 411 {bne}
>>      (int_list:REG_BR_PROB 4 (nil))
>>  -> 99)
> 
> This is an older compiler.  m68k no longer uses cc0 (except it is still
> mentioned in two comments (well, commented-out code)).
> 
>> >This is not unique to cc0 conversions: every port has a similar problem
>> >with all FIXED_REGISTERS.
>> 
>> It's not related to fixed registers.
> 
> No, it is exactly the same situation.  You cannot introduce uses of 
> such
> a register if it might already exist in the insn stream somewhere, not
> without checking first, and you better have a backup plan too.
> 
>> It's unique to CC registers since
>> these are on some plattforms modified by side effects. So after split2
>> it's modelled using CLOBBERs
> 
> There are no such implicit side effects if you have gotten rid of cc0.
> That is the *point* of removing cc0.
> 
>> >@findex cc0_rtx
>> >There is only one expression object of code @code{cc0}; it is the
>> >value of the variable @code{cc0_rtx}.  Any attempt to create an
>> >expression of code @code{cc0} will return @code{cc0_rtx}.
>> >
>> >There is a lot of code that depends on this property, you cannot break
>> >it without fixing everything.
>> 
>> There is no need to change the definition or modify any piece 
>> elsewhere.
>> And the modified comparison will still work for cc0.
> 
> Then you do not need your patch.  You can compare cc0_rtx by identity.
> 
> 
> Segher


since I still don't get it: i386.md expands cbranch into two insns, e.g.


(insn 17 16 18 4 (set (reg:CCNO 17 flags)
         (compare:CCNO (reg/v:SI 96 [ <retval> ])
             (const_int 0 [0]))) "x.c":2 3 {*cmpsi_ccno_1}
      (nil))
(jump_insn 18 17 19 4 (set (pc)
         (if_then_else (le (reg:CCNO 17 flags)
                 (const_int 0 [0]))
             (label_ref:DI 28)
             (pc))) "x.c":2 627 {*jcc_1}
      (int_list:REG_BR_PROB 1500 (nil))


What mechanism guarantees that no other insn is inserted inbetween the 
compare and the jump?

Or is i386 also "broken"?

Stefan

On Fri, Dec 13, 2019 at 6:56 PM Stefan Franke <stefan@franke.ms> wrote:
>
> Am 2019-12-13 21:59, schrieb Segher Boessenkool:
> > On Fri, Dec 13, 2019 at 08:55:15PM +0100, Stefan Franke wrote:
> >> Am 2019-12-13 18:58, schrieb Segher Boessenkool:
> >> >On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
> >> >>Why? If you are using a cc register plus your architecture as many
> >> >>instructions which may clobber that cc register, some passes (e.g.
> >> >>gcse)
> >> >>will reorder the insns. This can lead to the situation that an insn is
> >> >>moved between a compare and it' consuming jump insn. Which yields
> >> >>invalid code. (Note that at these stages clobbers are not yet tracked
> >> >>as
> >> >>CLOBBER insns).
> >> >
> >> >Then that port has a bug.  In the m68k port, there are no separate
> >> >compare
> >> >and jump insns ever, but for most ports those won't yet exist during
> >> >gcse.
> >>
> >> it looks like t2o insn for the m68k
> >>
> >> (insn 115 114 116 5 (set (cc0)
> >>         (compare (subreg:HI (reg:SI 272) 2)
> >>             (reg:HI 273)))
> >> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
> >> 17 {*m68k.md:559}
> >>      (nil))
> >> (jump_insn 116 115 117 5 (set (pc)
> >>         (if_then_else (ne (cc0)
> >>                 (const_int 0 [0]))
> >>             (label_ref 99)
> >>             (pc)))
> >> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
> >> 411 {bne}
> >>      (int_list:REG_BR_PROB 4 (nil))
> >>  -> 99)
> >
> > This is an older compiler.  m68k no longer uses cc0 (except it is still
> > mentioned in two comments (well, commented-out code)).
> >
> >> >This is not unique to cc0 conversions: every port has a similar problem
> >> >with all FIXED_REGISTERS.
> >>
> >> It's not related to fixed registers.
> >
> > No, it is exactly the same situation.  You cannot introduce uses of
> > such
> > a register if it might already exist in the insn stream somewhere, not
> > without checking first, and you better have a backup plan too.
> >
> >> It's unique to CC registers since
> >> these are on some plattforms modified by side effects. So after split2
> >> it's modelled using CLOBBERs
> >
> > There are no such implicit side effects if you have gotten rid of cc0.
> > That is the *point* of removing cc0.
> >
> >> >@findex cc0_rtx
> >> >There is only one expression object of code @code{cc0}; it is the
> >> >value of the variable @code{cc0_rtx}.  Any attempt to create an
> >> >expression of code @code{cc0} will return @code{cc0_rtx}.
> >> >
> >> >There is a lot of code that depends on this property, you cannot break
> >> >it without fixing everything.
> >>
> >> There is no need to change the definition or modify any piece
> >> elsewhere.
> >> And the modified comparison will still work for cc0.
> >
> > Then you do not need your patch.  You can compare cc0_rtx by identity.
> >
> >
> > Segher
>
>
> since I still don't get it: i386.md expands cbranch into two insns, e.g.
>
>
> (insn 17 16 18 4 (set (reg:CCNO 17 flags)
>          (compare:CCNO (reg/v:SI 96 [ <retval> ])
>              (const_int 0 [0]))) "x.c":2 3 {*cmpsi_ccno_1}
>       (nil))
> (jump_insn 18 17 19 4 (set (pc)
>          (if_then_else (le (reg:CCNO 17 flags)
>                  (const_int 0 [0]))
>              (label_ref:DI 28)
>              (pc))) "x.c":2 627 {*jcc_1}
>       (int_list:REG_BR_PROB 1500 (nil))
>
>
> What mechanism guarantees that no other insn is inserted inbetween the
> compare and the jump?

>(Note that at these stages clobbers are not yet tracked as
CLOBBER insns).

All of the instructions that need CLOBBER has it at this point.
So I think your back-end is not describing what it should be describing.
The old saying inside GCC is lie to reload and get wrong code.  That
rings true here too.

Thanks,
Andrew Pinski

>
> Or is i386 also "broken"?
>
> Stefan

Am 2019-12-14 04:03, schrieb Andrew Pinski:
> On Fri, Dec 13, 2019 at 6:56 PM Stefan Franke <stefan@franke.ms> wrote:
>> 
>> Am 2019-12-13 21:59, schrieb Segher Boessenkool:
>> > On Fri, Dec 13, 2019 at 08:55:15PM +0100, Stefan Franke wrote:
>> >> Am 2019-12-13 18:58, schrieb Segher Boessenkool:
>> >> >On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
>> >> >>Why? If you are using a cc register plus your architecture as many
>> >> >>instructions which may clobber that cc register, some passes (e.g.
>> >> >>gcse)
>> >> >>will reorder the insns. This can lead to the situation that an insn is
>> >> >>moved between a compare and it' consuming jump insn. Which yields
>> >> >>invalid code. (Note that at these stages clobbers are not yet tracked
>> >> >>as
>> >> >>CLOBBER insns).
>> >> >
>> >> >Then that port has a bug.  In the m68k port, there are no separate
>> >> >compare
>> >> >and jump insns ever, but for most ports those won't yet exist during
>> >> >gcse.
>> >>
>> >> it looks like t2o insn for the m68k
>> >>
>> >> (insn 115 114 116 5 (set (cc0)
>> >>         (compare (subreg:HI (reg:SI 272) 2)
>> >>             (reg:HI 273)))
>> >> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
>> >> 17 {*m68k.md:559}
>> >>      (nil))
>> >> (jump_insn 116 115 117 5 (set (pc)
>> >>         (if_then_else (ne (cc0)
>> >>                 (const_int 0 [0]))
>> >>             (label_ref 99)
>> >>             (pc)))
>> >> /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
>> >> 411 {bne}
>> >>      (int_list:REG_BR_PROB 4 (nil))
>> >>  -> 99)
>> >
>> > This is an older compiler.  m68k no longer uses cc0 (except it is still
>> > mentioned in two comments (well, commented-out code)).
>> >
>> >> >This is not unique to cc0 conversions: every port has a similar problem
>> >> >with all FIXED_REGISTERS.
>> >>
>> >> It's not related to fixed registers.
>> >
>> > No, it is exactly the same situation.  You cannot introduce uses of
>> > such
>> > a register if it might already exist in the insn stream somewhere, not
>> > without checking first, and you better have a backup plan too.
>> >
>> >> It's unique to CC registers since
>> >> these are on some plattforms modified by side effects. So after split2
>> >> it's modelled using CLOBBERs
>> >
>> > There are no such implicit side effects if you have gotten rid of cc0.
>> > That is the *point* of removing cc0.
>> >
>> >> >@findex cc0_rtx
>> >> >There is only one expression object of code @code{cc0}; it is the
>> >> >value of the variable @code{cc0_rtx}.  Any attempt to create an
>> >> >expression of code @code{cc0} will return @code{cc0_rtx}.
>> >> >
>> >> >There is a lot of code that depends on this property, you cannot break
>> >> >it without fixing everything.
>> >>
>> >> There is no need to change the definition or modify any piece
>> >> elsewhere.
>> >> And the modified comparison will still work for cc0.
>> >
>> > Then you do not need your patch.  You can compare cc0_rtx by identity.
>> >
>> >
>> > Segher
>> 
>> 
>> since I still don't get it: i386.md expands cbranch into two insns, 
>> e.g.
>> 
>> 
>> (insn 17 16 18 4 (set (reg:CCNO 17 flags)
>>          (compare:CCNO (reg/v:SI 96 [ <retval> ])
>>              (const_int 0 [0]))) "x.c":2 3 {*cmpsi_ccno_1}
>>       (nil))
>> (jump_insn 18 17 19 4 (set (pc)
>>          (if_then_else (le (reg:CCNO 17 flags)
>>                  (const_int 0 [0]))
>>              (label_ref:DI 28)
>>              (pc))) "x.c":2 627 {*jcc_1}
>>       (int_list:REG_BR_PROB 1500 (nil))
>> 
>> 
>> What mechanism guarantees that no other insn is inserted inbetween the
>> compare and the jump?
> 
>> (Note that at these stages clobbers are not yet tracked as
> CLOBBER insns).
> 
> All of the instructions that need CLOBBER has it at this point.
> So I think your back-end is not describing what it should be 
> describing.
> The old saying inside GCC is lie to reload and get wrong code.  That
> rings true here too.
> 
> Thanks,
> Andrew Pinski
> 


Thanks, now I do understand.

Stefan

On Fri, 2019-12-13 at 17:25 +0100, Stefan Franke wrote:
> Hi there,
> 
> I suggest this patch to allow architectures do substitute cc0_rtx with a 
> generated cc register.
> 
> Why? If you are using a cc register plus your architecture as many 
> instructions which may clobber that cc register, some passes (e.g. gcse) 
> will reorder the insns. This can lead to the situation that an insn is 
> moved between a compare and it' consuming jump insn. Which yields 
> invalid code. (Note that at these stages clobbers are not yet tracked as 
> CLOBBER insns).
> 
> To get around this behaviour you have to fake HAVE_CC0, e.g. by adding 
> some dummy code to your md file:
> 
>     (define_peephole2 [(set (match_operand 0 "" "") (cc0))] "" 
> [(const_int 0)] "")
> 
> plus some empty macros. All of this can be handled by the arch 
> implementation, but not the check for the cc0 inside of
> 
>    int sets_cc0_p(const_rtx x)
> 
> 
> Changelog
> 
>      gcc/jump.c: use rtx_equal do determine the identity of cc0_rtx
This is wrong.  cc0_rtx is a shared RTX expression.  There can be only
one.  Pointer equality is the right way to test, not rtx_equal_p. 
Furthermore on a cc0 machine the setter and user are always consecutive
in the RTL stream.  The optimizers know they're not supposed to
separate them.


Whatever you're doing, you're doing it wrong.

jeff
>

On Sat, 2019-12-14 at 03:55 +0100, Stefan Franke wrote:
> Am 2019-12-13 21:59, schrieb Segher Boessenkool:
> > On Fri, Dec 13, 2019 at 08:55:15PM +0100, Stefan Franke wrote:
> > > Am 2019-12-13 18:58, schrieb Segher Boessenkool:
> > > > On Fri, Dec 13, 2019 at 05:25:41PM +0100, Stefan Franke wrote:
> > > > > Why? If you are using a cc register plus your architecture as many
> > > > > instructions which may clobber that cc register, some passes (e.g.
> > > > > gcse)
> > > > > will reorder the insns. This can lead to the situation that an insn is
> > > > > moved between a compare and it' consuming jump insn. Which yields
> > > > > invalid code. (Note that at these stages clobbers are not yet tracked
> > > > > as
> > > > > CLOBBER insns).
> > > > 
> > > > Then that port has a bug.  In the m68k port, there are no separate
> > > > compare
> > > > and jump insns ever, but for most ports those won't yet exist during
> > > > gcse.
> > > 
> > > it looks like t2o insn for the m68k
> > > 
> > > (insn 115 114 116 5 (set (cc0)
> > >         (compare (subreg:HI (reg:SI 272) 2)
> > >             (reg:HI 273)))
> > > /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
> > > 17 {*m68k.md:559}
> > >      (nil))
> > > (jump_insn 116 115 117 5 (set (pc)
> > >         (if_then_else (ne (cc0)
> > >                 (const_int 0 [0]))
> > >             (label_ref 99)
> > >             (pc)))
> > > /tmp/compiler-explorer-compiler1191113-13975-1allrsj.w8mc/example.c:216
> > > 411 {bne}
> > >      (int_list:REG_BR_PROB 4 (nil))
> > >  -> 99)
> > 
> > This is an older compiler.  m68k no longer uses cc0 (except it is still
> > mentioned in two comments (well, commented-out code)).
> > 
> > > > This is not unique to cc0 conversions: every port has a similar problem
> > > > with all FIXED_REGISTERS.
> > > 
> > > It's not related to fixed registers.
> > 
> > No, it is exactly the same situation.  You cannot introduce uses of 
> > such
> > a register if it might already exist in the insn stream somewhere, not
> > without checking first, and you better have a backup plan too.
> > 
> > > It's unique to CC registers since
> > > these are on some plattforms modified by side effects. So after split2
> > > it's modelled using CLOBBERs
> > 
> > There are no such implicit side effects if you have gotten rid of cc0.
> > That is the *point* of removing cc0.
> > 
> > > > @findex cc0_rtx
> > > > There is only one expression object of code @code{cc0}; it is the
> > > > value of the variable @code{cc0_rtx}.  Any attempt to create an
> > > > expression of code @code{cc0} will return @code{cc0_rtx}.
> > > > 
> > > > There is a lot of code that depends on this property, you cannot break
> > > > it without fixing everything.
> > > 
> > > There is no need to change the definition or modify any piece 
> > > elsewhere.
> > > And the modified comparison will still work for cc0.
> > 
> > Then you do not need your patch.  You can compare cc0_rtx by identity.
> > 
> > 
> > Segher
> 
> since I still don't get it: i386.md expands cbranch into two insns, e.g.
> 
> 
> (insn 17 16 18 4 (set (reg:CCNO 17 flags)
>          (compare:CCNO (reg/v:SI 96 [ <retval> ])
>              (const_int 0 [0]))) "x.c":2 3 {*cmpsi_ccno_1}
>       (nil))
> (jump_insn 18 17 19 4 (set (pc)
>          (if_then_else (le (reg:CCNO 17 flags)
>                  (const_int 0 [0]))
>              (label_ref:DI 28)
>              (pc))) "x.c":2 627 {*jcc_1}
>       (int_list:REG_BR_PROB 1500 (nil))
> 
> 
> What mechanism guarantees that no other insn is inserted inbetween the 
> compare and the jump?
THose are not cc0.  Those are using a hard register in CC mode.

For x86, patterns which set/clobber the condition codes have explicit
sets/clobbers of the flags register.  As a result the dataflow is
accurately represented and the optimizers don't really have to do
anything special.  It's no different than cases there other hard
registers hold live data.


Contrast to a cc0 target (such as the H8 still in the tree).  On a cc0
target patterns which modify the condition codes do not describe them
at the RTL level (with the exception of cmp/tst insns).  Dataflow is
incomplete and as a result, most RTL passes have to handle things
specially to avoid the situation you're worried about.  Search for
HAVE_cc0.

Jeff

On Sun, Dec 15, 2019 at 03:45:34PM -0700, Jeff Law wrote:
> For x86, patterns which set/clobber the condition codes have explicit
> sets/clobbers of the flags register.  As a result the dataflow is
> accurately represented and the optimizers don't really have to do
> anything special.  It's no different than cases there other hard
> registers hold live data.

Yup.  And since x86 uses a fixed register as CC, already the earliest
passes cannot just create one anywhere.  Compare to if it used pseudos:
you can just create a fresh pseudo wherever you want, it is a new one,
nothing refers to it yet, nothing can conflict with it.

If you used pseudos although there is only one hard register that can be
used, you end up with quite a lot of save/restore code, and those are
typically expensive insns as well.  Quite a bit worse than the lost
optimisations you get from using a fixed register (insns cannot move as
much, lifetimes have to stay short, but that's about it).

> Contrast to a cc0 target (such as the H8 still in the tree).  On a cc0
> target patterns which modify the condition codes do not describe them
> at the RTL level (with the exception of cmp/tst insns).  Dataflow is
> incomplete and as a result, most RTL passes have to handle things
> specially to avoid the situation you're worried about.  Search for
> HAVE_cc0.

Only the insn immediately after a cc0 setter can use the value set (well,
ignoring branch delay slots.  Ignoring those is a great idea, unless you
are unlucky enough to have to use them ;-) ).  So it isn't hard to handle
the "dataflow" for them, in principle, but it is special cases
*everywhere*.


Segher