expand: Fix up edge splitting for GIMPLE_COND expansion if there are any PHIs [PR120629]

My r16-1398 PR120434 ranger during expansion change broke profiled lto
bootstrap on x86_64-linux, the following testcase is reduced from that.

The problem is during expand_gimple_cond, if we are unlucky that neither
of edge_true and edge_false point to the next basic block, the code
effectively attempts to split the false_edge and make the new bb BB_RTL
with some extra instructions which just arranges to jump.
It does it by creating a new bb, redirecting the false_edge and then
creating a new edge from the new bb to the dest.
Note, we don't have GIMPLE cfg hooks installed anymore and even if we
would, the 3 calls aren't the same as one split_edge with transformation
of the new bb into BB_RTL and adding it some BB_HEAD/BB_END.  If
false_edge->dest is BB_RTL or doesn't have PHI nodes (which before my
patch was always the case because), then this works fine, but with
PHI nodes on false_edge->dest redirect_edge_succ will remove the false_edge
from dest->preds (unordered remove which moves into its place the last edge
in the vector) and the new make_edge will then add the new edge as last
in the vector.  So, unless false_edge is the last edge in the dest->preds
vector this effectively swaps the last edge in the vector with
false_edge/its new replacement.
gimple_split_edge solves this by temporarily clearing phi_nodes on dest
(not needed when we don't have GIMPLE hooks), then making the new edge
first and redirecting the old edge (plus restoring phi_nodes on dest).
That way the redirection replaces the old edge with the new one and
PHI arguments don't need adjustment.  At the cost of temporarily needing
one more edge in the vector and so if unlucky reallocation.
Doing it like that is one of the options (i.e. just move the
make_single_succ_edge call).  This patch instead keeps doing what it did
and just swaps two edges again if needed to restore the PHI behavior
- remember edge_false->dest_idx first if there are PHI nodes in
edge_false->dest and afterwards if new edge's dest_idx is different from
the remembered one, swap the new edge with EDGE_PRED (dest, old_dest_idx).
That way PHI arguments are maintained properly as well.  Without this
we sometimes just swap PHI arguments.

In particular we had
  # ivtmp.24_52 = PHI <ivtmp.24_49(10), 1(6)>
on bb 8 (dest) and edge_false is the 10->8 edge.  We create a new
BB_RTL bb 15 on this edge, redirect the 10->8 edge to 10->15 which
does unordered_remove and so the bb8->preds edge vec is just 6->8,
PHIs not touched as in IR_RTL_CFGRTL mode.  Then a new 15->8 edge is
created.  Without the patch we get
  # ivtmp.24_52 = PHI <ivtmp.24_49(6), 1(15)>
which is wrong, while with this patch we get
  # ivtmp.24_52 = PHI <ivtmp.24_49(15), 1(6)>
which matches just the addition of (for ranger uninteresting) BB_RTL
on the 10->15->8 edge.

2025-06-12  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/120629
	* cfgexpand.cc (expand_gimple_cond): If dest bb isn't BB_RTL,
	has any PHI nodes and false_edge->dest_idx before redirection is
	different from make_single_succ_edge result's dest_idx, swap the
	latter with the former last pred edge and their dest_idx members.

	* g++.dg/opt/pr120629.C: New test.

diff --git a/gcc/cfgexpand.cc b/gcc/cfgexpand.cc
index 49b0b14042001a01a33f47a904000415acb27309..f288a54073462c38b0cb8044206ecb8aecf75272 100644 (file)
--- a/gcc/cfgexpand.cc
+++ b/gcc/cfgexpand.cc
@@ -3013,6 +3013,7 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
 
   new_bb = create_basic_block (NEXT_INSN (last), get_last_insn (), bb);
   dest = false_edge->dest;
+  unsigned int dest_idx = false_edge->dest_idx;
   redirect_edge_succ (false_edge, new_bb);
   false_edge->flags |= EDGE_FALLTHRU;
   new_bb->count = false_edge->count ();
@@ -3021,7 +3022,19 @@ expand_gimple_cond (basic_block bb, gcond *stmt)
   if (loop->latch == bb
       && loop->header == dest)
     loop->latch = new_bb;
-  make_single_succ_edge (new_bb, dest, 0);
+  edge e = make_single_succ_edge (new_bb, dest, 0);
+  if ((dest->flags & BB_RTL) == 0
+      && phi_nodes (dest)
+      && e->dest_idx != dest_idx)
+    {
+      /* If there are any PHI nodes on dest, swap the new succ edge
+        with the one moved into false_edge's former position, so that
+        PHI arguments don't need adjustment.  */
+      edge e2 = EDGE_PRED (dest, dest_idx);
+      std::swap (e->dest_idx, e2->dest_idx);
+      std::swap (EDGE_PRED (dest, e->dest_idx),
+                EDGE_PRED (dest, e2->dest_idx));
+    }
   if (BARRIER_P (BB_END (new_bb)))
     BB_END (new_bb) = PREV_INSN (BB_END (new_bb));
   update_bb_for_insn (new_bb);

diff --git a/gcc/testsuite/g++.dg/opt/pr120629.C b/gcc/testsuite/g++.dg/opt/pr120629.C
new file mode 100644 (file)
index 0000000..70a9cdd
--- /dev/null
+++ b/gcc/testsuite/g++.dg/opt/pr120629.C
@@ -0,0 +1,53 @@
+// PR middle-end/120629
+// { dg-do run }
+// { dg-options "-O2 -fprofile-generate -fno-exceptions -fno-rtti" }
+// { dg-require-profiling "-fprofile-generate" }
+
+__attribute__((noipa, noreturn, cold)) void
+foo (const char *, int, const char *)
+{
+  __builtin_abort ();
+}
+
+struct S
+{
+  __attribute__((noipa)) void bar (void *);
+  static const int a = 8;
+  unsigned int b[a + 1];
+};
+
+__attribute__((noipa)) unsigned long
+baz (void *)
+{
+  static int x = 8;
+  return --x;
+}
+
+__attribute__((noipa)) void
+S::bar (void *x)
+{
+  unsigned int c;
+  int k = 0;
+
+  do
+    {
+      ((void) (!(k <= a) ? foo ("foo", 42, __FUNCTION__), 0 : 0));
+      c = b[k++] = baz (x);
+    }
+  while (c);
+  while (k <= a)
+    b[k++] = 0;
+}
+
+int
+main ()
+{
+  struct T { S a; unsigned int b; } s = {};
+  s.b = 0x1234;
+  s.a.bar (0);
+  for (int i = 0; i < 9; ++i)
+    if (s.a.b[i] != (i == 8 ? 0 : 7 - i))
+      __builtin_abort ();
+  if (s.b != 0x1234)
+    __builtin_abort ();
+}