diff mbox series

scev: Avoid ICE on results used in abnormal PHI args [PR113201]

Message ID ZZZ7vGAu19HuMlge@tucnak
State Unresolved
Headers
Series scev: Avoid ICE on results used in abnormal PHI args [PR113201] |

Checks

Context Check Description
snail/gcc-patch-check warning Git am fail log

Commit Message

Jakub Jelinek Jan. 4, 2024, 9:34 a.m. UTC 
  Hi!

The following testcase ICEs when rslt is SSA_NAME_OCCURS_IN_ABNORMAL_PHI
and we call replace_uses_by with a INTEGER_CST def, where it ICEs on:
              if (e->flags & EDGE_ABNORMAL
                  && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (val))
because val is not an SSA_NAME.  One way would be to add
		  && TREE_CODE (val) == SSA_NAME
check in between the above 2 lines in replace_uses_by.

And/or the following patch just punts propagating constants to
SSA_NAME_OCCURS_IN_ABNORMAL_PHI rslt uses.

Or we could punt somewhere earlier in final value replacement (but dunno
where).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or do you want the replace_uses_by change (or both that and the following,
something else)?

2024-01-04  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/113201
	* tree-scalar-evolution.cc (final_value_replacement_loop): Don't call
	replace_uses_by on SSA_NAME_OCCURS_IN_ABNORMAL_PHI rslt.

	* gcc.c-torture/compile/pr113201.c: New test.


	Jakub

Comments

Jeff Law Jan. 4, 2024, 9:41 p.m. UTC | #1 
On 1/4/24 02:34, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase ICEs when rslt is SSA_NAME_OCCURS_IN_ABNORMAL_PHI
> and we call replace_uses_by with a INTEGER_CST def, where it ICEs on:
>                if (e->flags & EDGE_ABNORMAL
>                    && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (val))
> because val is not an SSA_NAME.  One way would be to add
> 		  && TREE_CODE (val) == SSA_NAME
> check in between the above 2 lines in replace_uses_by.
> 
> And/or the following patch just punts propagating constants to
> SSA_NAME_OCCURS_IN_ABNORMAL_PHI rslt uses.
> 
> Or we could punt somewhere earlier in final value replacement (but dunno
> where).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Or do you want the replace_uses_by change (or both that and the following,
> something else)?
> 
> 2024-01-04  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR tree-optimization/113201
> 	* tree-scalar-evolution.cc (final_value_replacement_loop): Don't call
> 	replace_uses_by on SSA_NAME_OCCURS_IN_ABNORMAL_PHI rslt.
> 
> 	* gcc.c-torture/compile/pr113201.c: New test.
I think we have to punt somewhere -- we can't insert the constant 
initialization on an abnormal edge.  So OK for trunk.

jeff
diff mbox series

Patch

--- gcc/tree-scalar-evolution.cc.jj	2024-01-03 11:51:38.929628514 +0100
+++ gcc/tree-scalar-evolution.cc	2024-01-03 18:31:17.550973344 +0100
@@ -3881,7 +3881,7 @@  final_value_replacement_loop (class loop
 
       /* Propagate constants immediately, but leave an unused initialization
 	 around to avoid invalidating the SCEV cache.  */
-      if (CONSTANT_CLASS_P (def))
+      if (CONSTANT_CLASS_P (def) && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (rslt))
 	replace_uses_by (rslt, def);
 
       /* Create the replacement statements.  */
--- gcc/testsuite/gcc.c-torture/compile/pr113201.c.jj	2024-01-03 18:44:18.859121266 +0100
+++ gcc/testsuite/gcc.c-torture/compile/pr113201.c	2024-01-03 18:37:28.099814475 +0100
@@ -0,0 +1,15 @@ 
+/* PR tree-optimization/113201 */
+
+void foo (void) __attribute__((returns_twice));
+void bar (void);
+
+int
+baz (void)
+{
+  int x = 42;
+  foo ();
+  while (--x)
+    ;
+  bar ();
+  return x;
+}