Am 2019-12-14 04:03, schrieb Andrew Pinski:
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



Thanks, now I do understand.

Stefan

Reply via email to