public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug rtl-optimization/93264] [10 Regression] ICE in cfg_layout_redirect_edge_and_branch_force, at cfgrtl.c:4522
Date: Fri, 03 Apr 2020 08:12:10 +0000	[thread overview]
Message-ID: <bug-93264-4-hpxLyKhK6A@http.gcc.gnu.org/bugzilla/> (raw)
In-Reply-To: <bug-93264-4@http.gcc.gnu.org/bugzilla/>

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93264

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Roman Zhuykov from comment #6)
> First, I want here to mention that Richard have recently discussed
> partitioning in mailing list with Segher, starting from
> https://gcc.gnu.org/ml/gcc-patches/2020-02/msg00666.html
> 
> > I'll run some cross-testing to check how it works for now.
> Second, those tests have finished and there is nothing to say about the
> results. From correctness aspect everything looks fine.  But I haven't tried
> to look at each example where it prevents/allows loops to be scheduled, and
> know nothing about performance impact.
> 
> (In reply to rsandifo@gcc.gnu.org from comment #4)
> > Yeah, it ought to be better to do mode switching first.  But I think
> > the more important ones are:
> >
> >        NEXT_PASS (pass_split_all_insns);
> >        NEXT_PASS (pass_lower_subreg3);
> >
> > Scheduling should happen on the split form of insns rather than the
> > unsplit form.  lower_subreg should also improve "schedulability".
> Agreed, so it would be much better to fix the issue conservatively without
> moving the pass.

As of pass ordering partitioning needs to run before RA but I don't see
why it necessarily needs to run that early.  Well, it works on CFG layout
mode and it's the "last" pass before we go out of CFG layout - but obviously
SMS goes back into so we could do the very same dance for partitioning
and only schedule it after SMS (or even later).

The whole complication arises from the direct CFG edges that are
implemented with indirect jumps and which we cannot easily redirect.
A more high-level representation of those "direct jumps" would make
redirection possible again (at the expense of micro-optimizing the
reg set + indirect jump between partitioning and the point we
expose the operation).  The only thing we need is the RA giving us
a scratch register we can use.  Maybe we can have a special named
pattern for those jumps allocating an extra scratch and have targets
either split them after reload into true indirect jumps or leave them
in place all the way to asm output.  So they'd appear as simple_jump_p
for a much longer time?

Currently we have

(insn 577 350 578 27 (set (reg:DI 309)
        (high:DI (label_ref:DI 220))) "pr71550.c":19:20 -1
     (insn_list:REG_LABEL_OPERAND 220 (nil)))
(insn 578 577 579 27 (set (reg:DI 308)
        (lo_sum:DI (reg:DI 309)
            (label_ref:DI 220))) "pr71550.c":19:20 -1
     (insn_list:REG_LABEL_OPERAND 220 (expr_list:REG_EQUAL (label_ref:DI 220)
            (nil))))
(jump_insn/j 579 578 220 27 (set (pc)
        (reg:DI 308)) "pr71550.c":19:20 -1
     (nil)

not sure if jump_insns can have extra operands for the scratch but
we want part of the jump to be like

(jump_insn/j 579 578 220 27 (set (pc)
        (label_ref:DI 220)) "pr71550.c":19:20 -1
     (nil)

but also mention (reg:DI 309) as scratch.

Ideas?  I guess doing

(jump_insn/j 579 578 220 27 [
  (set (pc) (label_ref:DI 220))
  (clobber (reg:DI 309))])

might work?  The pattern of the jump insn woudl be a parallel though
so not match simplejump_p.  There's also JUMP_LABEL, not sure if that's
usable.  There's condjump_in_parallel_p so we could add
simplejump_in_parallel_p as well and use that in CFG manipulation.

This woudl of course need support from targets but where partitioning
decides it needs to emit indirect jumps it could say that it needs
such target support or otherwise refuse to operate.  Currently
we look at HAS_LONG_COND_BRANCH / HAS_LONG_UNCOND_BRANCH where
only failure in the latter needs indirect jumps using "indirect_jump".
So we'd add sth like

(define_insn "crossing_jump"
  [(set (pc) (label_ref (match_operand 0 "" ""))
   (clobber (match_scratch:DI 1))]
  ""
  "br\\t%1"
  [(set_attr "type" "branch")]
)

with the assembly part adjusted accordingly (the move is missing).  As said
a late splitter may be the best approach (but that's up to the target).

And partitioning would require either HAS_LONG_UNCOND_BRANCH or
targetm.have_crossing_jump and we'd arrange for CFG manipulation to
handle this form.
Oh, and while these issues can trigger in a way appearing as P1 regression
they of course really are not new issues.  So yeah, not really P1.

  parent reply	other threads:[~2020-04-03  8:12 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <bug-93264-4@http.gcc.gnu.org/bugzilla/>
2020-04-02 11:44 ` rguenth at gcc dot gnu.org
2020-04-02 12:10 ` jakub at gcc dot gnu.org
2020-04-02 12:26 ` rguenth at gcc dot gnu.org
2020-04-02 12:32 ` rguenth at gcc dot gnu.org
2020-04-02 13:55 ` zhroma at gcc dot gnu.org
2020-04-03  8:12 ` rguenth at gcc dot gnu.org [this message]
2020-04-03  8:15 ` rguenth at gcc dot gnu.org
2020-04-03 10:13 ` jakub at gcc dot gnu.org
2020-04-03 10:21 ` jakub at gcc dot gnu.org
2020-04-09 12:20 ` rguenth at gcc dot gnu.org
2021-04-27 11:38 ` [Bug rtl-optimization/93264] [10/11/12 " jakub at gcc dot gnu.org
2021-07-28  7:04 ` rguenth at gcc dot gnu.org
2022-04-21  7:47 ` rguenth at gcc dot gnu.org
2023-05-29 10:02 ` [Bug rtl-optimization/93264] [10/11/12/13/14 " jakub at gcc dot gnu.org

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bug-93264-4-hpxLyKhK6A@http.gcc.gnu.org/bugzilla/ \
    --to=gcc-bugzilla@gcc.gnu.org \
    --cc=gcc-bugs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).