On Tue, Oct 29, 2019 at 8:34 PM Jeff Law <l...@redhat.com> wrote:
>
> 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.
I think the wiki has better examples. That said, I wonder how much can
be automated here, for example when just considering CCmode (m68k has
setcc IIRC) then for example all define_insns like
(define_insn "*cmpsi_cf"
[(set (cc0)
(compare (match_operand:SI 0 "nonimmediate_operand" "mrKs,r")
(match_operand:SI 1 "general_operand" "r,mrKs")))]
"TARGET_COLDFIRE"
{...}
could be rewritten based on seeing (set (cc0) ...) to a (set (reg:CC
CC_REG) ...)
plus adding a "&& reload_completed" condition. That could be done
transparently by the .md file reader even?
The various expanders need to be handled manually I guess, though somehow
automagically generating the define_insn_and_split might be possible
as well, no?
That is, the work is quite repetitive (looking at m68k.md) and for this reason
error-prone. But a case#2 conversion could essentially be automated?
That is, do you see it as impossible to implement a case#2 cc0 "conversion"
in gen*?
> 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.
I don't see that it does this currently though (cc0 doesn't allow this).
> 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:
For m68k what helps is that you could concentrate on TARGET_COLDFIRE
(or !TARGET_COLDFIRE). Maybe even split m68k.md along this.
A lot of the define_insns also look like optimization (so could be disabled
for the moment).
> (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
>
