Fix combiner ICE with -g (PR rtl-optimization/85300)

Message ID 20180410135033.GG8577@tucnak
State New
Headers show
Series
  • Fix combiner ICE with -g (PR rtl-optimization/85300)
Related show

Commit Message

Jakub Jelinek April 10, 2018, 1:50 p.m.
Hi!

The following testcase ICEs, because subst first creates invalid RTL
(in this case (float:SF (const_int 256))) and only later on simplifies it
using on the-side op0_mode.  In the end try_combine succeeds, so we don't
undo anything from the undo buffer, but call propagate_for_debug with the
patterns that were partially modified by subst and ICE on those.

Most unary ops should be ok, in particular those where the operand must have
either the same mode as the whole op, or can be VOIDmode which is assumed to
be the op's mode then.  Exceptions are unary ops that have different modes,
including SUBREG (not really unary but extra), {ZERO,SIGN}_EXTEND, TRUNCATE,
FLOAT and UNSIGNED_FLOAT.  SIGN_EXTEND is by some code considered ok because
the constants must be sign-extended anyway, TRUNCATE can be handled just by
truncating the constant regardless of whatever mode it could have had;
in theory we could also handle FLOAT better than we do right now:
      if (op_mode == VOIDmode)
        {
          /* CONST_INT have VOIDmode as the mode.  We assume that all
             the bits of the constant are significant, though, this is
             a dangerous assumption as many times CONST_INTs are
             created and used with garbage in the bits outside of the
             precision of the implied mode of the const_int.  */
          op_mode = MAX_MODE_INT;
        }
just doesn't work right, MAX_MODE_INT in this case causes the ICE, because
it is way too large.  We could instead just try to iterate from narrowest
integral mode and check if trunc_int_for_mode (INTVAL (), op_mode) == INTVAL ()
and stop at the narrowest mode where this is true.  But for ZERO_EXTEND and
UNSIGNED_FLOAT when we don't know the original mode we just can't recover
from it.

subst already has code to deal with ZERO_EXTEND, this patch just handles
FLOAT and UNSIGNED_FLOAT the same; that way we don't create the invalid
RTL.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2018-04-10  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/85300
	* combine.c (subst): Handle subst of CONST_SCALAR_INT_P new_rtx also
	into FLOAT and UNSIGNED_FLOAT like ZERO_EXTEND, return a CLOBBER if
	simplify_unary_operation fails.

	* gcc.dg/pr85300.c: New test.


	Jakub

Comments

Segher Boessenkool April 10, 2018, 3:17 p.m. | #1
Hi Jakub,

On Tue, Apr 10, 2018 at 03:50:33PM +0200, Jakub Jelinek wrote:
> The following testcase ICEs, because subst first creates invalid RTL
> (in this case (float:SF (const_int 256))) and only later on simplifies it
> using on the-side op0_mode.  In the end try_combine succeeds, so we don't
> undo anything from the undo buffer, but call propagate_for_debug with the
> patterns that were partially modified by subst and ICE on those.

> subst already has code to deal with ZERO_EXTEND, this patch just handles
> FLOAT and UNSIGNED_FLOAT the same; that way we don't create the invalid
> RTL.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Yes please.  And thanks for the explanation!


Segher


> 2018-04-10  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/85300
> 	* combine.c (subst): Handle subst of CONST_SCALAR_INT_P new_rtx also
> 	into FLOAT and UNSIGNED_FLOAT like ZERO_EXTEND, return a CLOBBER if
> 	simplify_unary_operation fails.
> 
> 	* gcc.dg/pr85300.c: New test.

Patch

--- gcc/combine.c.jj	2018-04-09 20:15:43.704632013 +0200
+++ gcc/combine.c	2018-04-10 09:06:25.826606396 +0200
@@ -5575,11 +5575,15 @@  subst (rtx x, rtx from, rtx to, int in_d
 		    x = gen_rtx_CLOBBER (mode, const0_rtx);
 		}
 	      else if (CONST_SCALAR_INT_P (new_rtx)
-		       && GET_CODE (x) == ZERO_EXTEND)
+		       && (GET_CODE (x) == ZERO_EXTEND
+			   || GET_CODE (x) == FLOAT
+			   || GET_CODE (x) == UNSIGNED_FLOAT))
 		{
-		  x = simplify_unary_operation (ZERO_EXTEND, GET_MODE (x),
-						new_rtx, GET_MODE (XEXP (x, 0)));
-		  gcc_assert (x);
+		  x = simplify_unary_operation (GET_CODE (x), GET_MODE (x),
+						new_rtx,
+						GET_MODE (XEXP (x, 0)));
+		  if (!x)
+		    return gen_rtx_CLOBBER (VOIDmode, const0_rtx);
 		}
 	      else
 		SUBST (XEXP (x, i), new_rtx);
--- gcc/testsuite/gcc.dg/pr85300.c.jj	2018-04-10 08:53:04.594782804 +0200
+++ gcc/testsuite/gcc.dg/pr85300.c	2018-04-10 08:53:04.594782804 +0200
@@ -0,0 +1,16 @@ 
+/* PR rtl-optimization/85300 */
+/* { dg-do compile } */
+/* { dg-options "-O1 -g -funroll-all-loops -fno-tree-ter -fno-web" } */
+
+void
+foo (double x, unsigned char y)
+{
+  while ((int) x < 1)
+    {
+      float a;
+
+      a = y | 0x100;
+      y = 0;
+      x = a;
+    }
+}