On Fri, Dec 13, 2019 at 6:56 PM Stefan Franke <ste...@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
