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

            Bug ID: 90040
           Summary: [meta-bug] modulo-scheduler and partitioning issues
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhroma at ispras dot ru
  Target Milestone: ---

Here I want to discuss the situation with modulo scheduler pass when
-freorder-blocks-and-partition is also enabled.

TL;DR Kindly ask RTL folks to fix ICE happening in 
cfg_layout_redirect_edge_and_branch_force while trying to redirect a crossing
jump.

--
The problem here is not in modulo scheduler algorithm itself, but in pass
initialization (and finalization) procedures. It enter (and later finally
leaves) cfg_layout mode, it also calls loop_optimizer_init. And all this stuff
leads to many branch redirections, which should successfully happen after
partitioning in bbro pass.

This issue is not new, at least here
https://gcc.gnu.org/ml/gcc-patches/2011-07/msg01811.html
I found that entering cfg_layout broke things on x86 (where required doloop
pattern is absent), and introduce an idea to move the sms pass before bbro. But
later I got this thought from Richard
https://gcc.gnu.org/ml/gcc-patches/2011-10/msg01526.html
and I agree that we have to fix it in another way.

Now in 2019, we got at least five bugs about the same situation. I want to
connect them all into this discussion.

First to say, PR85408 and PR87329 are open now, PR87475 is fixed, but they all
are about the same assertion
«internal compiler error: in patch_jump_insn, at cfgrtl.c:1271». PR87475 was
fixed by Jakub back in November by r266219, and two other were later reported
unreproducible, so, IMHO, they are fixed now too.

Jakub's ChangeLog:
        PR rtl-optimization/87475
        * cfgrtl.c (patch_jump_insn): Allow redirection failure for
CROSSING_JUMP_P insns.
        (cfg_layout_redirect_edge_and_branch): Don't ICE if ret is NULL.

Second, I want to discuss PR85426, where first buggy assertion was same as in
that three PRs, but after Jakub’s fix, Arseny reports another fallen assertion:
«internal compiler error: in cfg_layout_redirect_edge_and_branch_force, at
cfgrtl.c:4482»

And this last assertion is the real issue -- that happens when we call
redirect_edge_and_branch_force for crossing jump. I’m not familiar with
partitioning, so have no idea how to fix this.

Hopefully, if we find a fix for this, PR89116 would also be fixed, although
situation there in not absolutely the same — there we ICE on same assertion
only in split_edge_and_insert while running generate_prolog_epilog, so this
happens only after SMS succeeded to create a schedule.

[Appendix 1] I want to mention PR83886 also, which was fixed by Honza with the
following ChangeLog:
        PR rtl/84058
        * cfgcleanup.c (try_forward_edges): Do not give up on crossing
        jumps; choose last target that matches the criteria (i.e.
        no partition changes for non-crossing jumps).
        * cfgrtl.c (cfg_layout_redirect_edge_and_branch): Add basic
        support for redirecting crossing jumps to non-crossing.

So here we have also some change with crossing jumps redirection.

[Appendix 2] There are also some issues with entering/exiting cfg_layout.
Technically, all of them are fixed right now: PR83771, PR83475 (fixed together
with PR81791).
But I wonder maybe this all is just latent, because in other passes I see a
special check to prevent entering cfg_layout after partitioning.

For example in hw-doloop.c we have:
  /* We can't enter cfglayout mode anymore if basic block partitioning
     already happened.  */
  if (do_reorder && !crtl->has_bb_partition)

This condition were added back in 2014
https://gcc.gnu.org/ml/gcc-patches/2014-01/msg00282.html and that time it looks
like:
  if (do_reorder && !flag_reorder_blocks_and_partition)

And later were adjusted by Honza to current state:
https://gcc.gnu.org/ml/gcc-patches/2017-06/msg00515.html

But we dont have any check like this in modulo scheduler. There we certainly
can’t proceed without entering cfg_layout, so I’m not sure it would be a good
idea to add such a check. But maybe we have now some more latent bugs with
entering cfg_layout after partitioning?

[Appendix 3] Last month I spent a lot of time updating my patches described
here
https://gcc.gnu.org/ml/gcc-patches/2017-02/msg01647.html
and have locally added several other patches to fix most of modulo-sched PRs
from bugzilla, but annoying issue described here prohibits me to test my branch
in all possible scenarios.
I'll try to add some comments into all other modulo scheduler bugzilla PRs this
or next week while we are on stage 4.

Reply via email to