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

Reply via email to