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
