https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123994
Jeffrey A. Law <law at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rsandifo at gcc dot gnu.org
--- Comment #9 from Jeffrey A. Law <law at gcc dot gnu.org> ---
Andrew, it's not clear to me we can compare insns like that in the rtl-ssa
framework. Those operators providing an ordering, but I don't think it's
always a global ordering.
// Compare instructions by their positions in the function list described
// above. Thus for two instructions in the same basic block, I1 < I2 if
// I1 comes before I2 in the block.
bool operator< (const insn_info &) const;
bool operator<= (const insn_info &) const;
bool operator>= (const insn_info &) const;
bool operator> (const insn_info &) const;
Let's assume the basic idea is that in the absence of loops if A > B, then A
occurs after B. But there's not a viable global order in the presense of a
irreducible loop -- which this test case has by the time we get into
late-combine. I was a bit surprised by the irreducible loop, so I verified it
by drawing the CFG by hand. Sure enough, it's got an irredicble loop.
Note that I dn't think you can move insn 111 after insn 40. You'd also have to
move insn 112 and insn 113. Moving insn 113 would change semantics. Not to
mention you probably don't want to move anything from the exit path into a loop
:-)
So the natural question is what next. ISTM that if the loop doesn't find a
suitable location (min_insn goes to NULL), then we need to exit the loop and
return false with a diagnostic into the dump file.
So I come up with this:
diff --git a/gcc/rtl-ssa/changes.cc b/gcc/rtl-ssa/changes.cc
index e0df982880c..8afe8ea4eb3 100644
--- a/gcc/rtl-ssa/changes.cc
+++ b/gcc/rtl-ssa/changes.cc
@@ -110,9 +110,10 @@ function_info::verify_insn_changes
(array_slice<insn_change *const> changes)
// Make sure that the changes can be kept in their current order
// while honoring all of the move ranges.
min_insn = later_insn (min_insn, change->move_range.first);
- while (min_insn != change->insn () && !can_insert_after (min_insn))
+ while (min_insn && min_insn != change->insn () && !can_insert_after
(min_insn))
min_insn = min_insn->next_nondebug_insn ();
- if (*min_insn > *change->move_range.last)
+
+ if (!min_insn || *min_insn > *change->move_range.last)
{
if (dump_file && (dump_flags & TDF_DETAILS))
fprintf (dump_file, "no viable insn position assignment\n");
But having never worked in this code, I don't have a high degree of confidence
in the patch. I've CC'd Richard S in the hopes that he'll take pity on us and
take a looksie.
