Re: Deprecating cc0 (and consequently cc0 targets)

[Jeff Law <law at redhat dot com>]

On 10/29/19 6:26 AM, John Paul Adrian Glaubitz wrote:
> Hello!
> 
> We have raised $5000 to support anyone willing to work on this for the
> m68k target [1]. We really need the m68k to stay as it's essential to
> be able to compile for Linux/m68k, NetBSD/m68k and AROS/m68k (a new
> and ABI-compatible AmigaOS clone). I'm sure other communities like in the
> NeXTStep, Atari and Mac/m68k forums are interested in keeping the backend
> as well. I'm confident we will be able to raise even more money as the
> community is large and active.
> 
>> If someone is interested in possibly doing a conversion, I would
>> strongly recommend reviewing the best documentation we have on the
>> subject:
>>
>> https://gcc.gnu.org/wiki/CC0Transition
>>
>> I worked with my son about a year ago to transition the v850 to MODE_CC
>> using that page as a guideline.  We also have a partial transition for
>> the H8/300 port (not far enough to even build anything yet).  I had
>> hoped we would finish the H8/300 port last year and would have tackled
>> another, but events have gotten in the way.
> 
> Any chance that the unfinished code can be shared? I'm looking for any
> blueprints that can be used as a guidance for the work on the m68k
> backend.
I'm not sure it'd be all that useful.  The v850 bits would be a better
starting point.  The partial H8 transition isn't committed anywhere
because it would certainly break the port.

> 
> I have also created a guide on how to set up a QEMU virtual machine running
> a fully fledged Debian/m68k which can be used for the development work [2].
> 
>> The transition isn't terribly hard, but does take time because of the
>> number of patterns that have to be changed.
> 
> I have already looked at the conversion done on the V850 backend [3] but so far
> I don't understand enough of the register representation stuff to be able
> to know where to start. I'm seeing that all the "cc" attributes are being removed
> but not replaced?
Right. The cc attributes were used to describe which condition codes
were valid after each insn.  That information now has to be carried by
the mode of the condition code register.

I'll walk you through one example from the H8 port.  This isn't complete
enough to even test that it builds.


> (define_insn "*addsi_h8300"
>   [(set (match_operand:SI 0 "register_operand" "=r,r")
>         (plus:SI (match_operand:SI 1 "register_operand" "%0,0")
>                  (match_operand:SI 2 "h8300_src_operand" "n,r")))]
>   "TARGET_H8300"
> {
>   return output_plussi (operands);
> }
>   [(set (attr "length")
>         (symbol_ref "compute_plussi_length (operands)"))
>    (set (attr "cc")
>         (symbol_ref "compute_plussi_cc (operands)"))])


That needs to turn into a define_insn_and_split.  The idea is the form
above is what's seen prior to register allocation and reloading.  After
reloading we turn it into a form with an attached clobber.  We achieve
this by rewriting the pattern into a define_insn_and_split like this:

> (define_insn_and_split "*addsi_h8300"
>    [(set (match_operand:SI 0 "register_operand" "=r,r")
>         (plus:SI (match_operand:SI 1 "register_operand" "%0,0")
>                  (match_operand:SI 2 "h8300_src_operand" "n,r")))]
>   "TARGET_H8300"
>   "#"
>   "reload_completed"
>   [(parallel [(set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))
>               (clobber (reg:CC CC_REG))])])

We added the "#" which indicates the pattern has to be split and will
never generate assembly code in and of itself.  The "reload_completed"
indicates when the spliter should fire (after reload).  The subsequent
RTL is how the pattern should be rewritten after reload (adds the clobber).

Then you need a new pattern which matches the output of the splitter
with the attached clobber.  Something like this:

> (define_insn "*addsi_h8300_clobber_flags"
>   [(set (match_operand:SI 0 "register_operand" "=r,r")
>        (plus:SI (match_operand:SI 1 "register_operand" "%0,0")
>                 (match_operand:SI 2 "h8300_src_operand" "n,r")))
>    (clobber (reg:CC CC_REG))]
>   "TARGET_H8300"
>  {
>    return output_plussi (operands);
>  }
This is the form that we'll use after reload.  It explicitly clobbers
the condition codes which is the key here.

Some have claimed this might be easier with define_subst, but I wandered
it a bit and it didn't seem necessarily simpler in the end.  But maybe
someone with more experience with define_subst could make the transition
simpler.

Anyway, that has to be done for each and every pattern that modifies the
condition codes.  It's a ton of mechanical work.  The m68k has the
additional complication that condition codes can be either in cc0 or
fcc0.  So that has to be taken into account as well.

You also have to rewrite the tst/cmp and conditional branching patterns
to use the new style.

Sadly until you do *all* of that you don't have anything you can really
test.  And even once you've done all that, the code quality is going to
suffer because we're not doing any compare/tst elimination.  To enable
that you have to go back and start adding patterns like this:


(define_insn "*addsi_h8300_set_flags"
  [(set (reg:CCNZ CC_REGNUM)
         (compare CCNZ (plus:SI (match_operand:SI 1 "register_operand"
"%0,0")
                                (match_operand:SI 2 "register_operand"
"n, r"))))
    (set (match_operand:SI 0 "register_operand" "=r,r")
         (plus:SI (match_dup 1) (match_dup 2)))]
  "reload_completed && TARGET_H8300"
 {
   return output_plussi (operands);
 }

This is where we describe (via the mode of the condition code register)
what condition codes are set and how to extract them.  These patterns
can be added incrementally.  But again, it's going to be a ton of work.

My son is definitely still interested.  But I doubt he'd likely be able
to take up the task until the spring/summer of 2020.  Additionally,
there's some question of whether or not him doing the work could be
viewed as a conflict of interest WRT the bug bounty because of his
relationship to me.


jeff