Patchwork Remove introduction of undefined overflow in emit_case_bit_test.

login
register
mail settings
Submitter Tom de Vries
Date Aug. 5, 2012, 11:31 p.m.
Message ID <501F0265.9090300@mentor.com>
Download mbox | patch
Permalink /patch/175219/
State New
Headers show

Comments

Tom de Vries - Aug. 5, 2012, 11:31 p.m.
Richard,

the code in emit_case_bit_tests currently introduces a MINUS_EXPR in signed type
(without checking if signed types wrap or not), which could mean introduction of
undefined overflow.

This patch fixes this problem by performing the MINUS_EXPR in an unsigned type.

Doing so also enables the vrp optimization for the test-case.

Bootstrapped and reg-tested (ada inclusive) on x86_64.

OK for trunk?

Thanks,
- Tom


2012-08-06  Tom de Vries  <tom@codesourcery.com>

	* tree-switch-conversion.c (emit_case_bit_tests): Generate MINUS_EXPR in
	unsigned type.

	* gcc.dg/tree-ssa/vrp78.c: New test.
Richard Guenther - Aug. 6, 2012, 8:17 a.m.
On Mon, Aug 6, 2012 at 1:31 AM, Tom de Vries <Tom_deVries@mentor.com> wrote:
> Richard,
>
> the code in emit_case_bit_tests currently introduces a MINUS_EXPR in signed type
> (without checking if signed types wrap or not), which could mean introduction of
> undefined overflow.
>
> This patch fixes this problem by performing the MINUS_EXPR in an unsigned type.
>
> Doing so also enables the vrp optimization for the test-case.
>
> Bootstrapped and reg-tested (ada inclusive) on x86_64.
>
> OK for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> - Tom
>
>
> 2012-08-06  Tom de Vries  <tom@codesourcery.com>
>
>         * tree-switch-conversion.c (emit_case_bit_tests): Generate MINUS_EXPR in
>         unsigned type.
>
>         * gcc.dg/tree-ssa/vrp78.c: New test.

Patch

Index: gcc/tree-switch-conversion.c
===================================================================
--- gcc/tree-switch-conversion.c (revision 190139)
+++ gcc/tree-switch-conversion.c (working copy)
@@ -384,10 +384,10 @@  emit_case_bit_tests (gimple swtch, tree
 
   gsi = gsi_last_bb (switch_bb);
 
-  /* idx = (unsigned) (x - minval) */
-  idx = fold_build2 (MINUS_EXPR, index_type, index_expr,
-		     fold_convert (index_type, minval));
-  idx = fold_convert (unsigned_index_type, idx);
+  /* idx = (unsigned)x - minval */
+  idx = fold_convert (unsigned_index_type, index_expr);
+  idx = fold_build2 (MINUS_EXPR, unsigned_index_type, idx,
+		     fold_convert (unsigned_index_type, minval));
   idx = force_gimple_operand_gsi (&gsi, idx,
 				  /*simple=*/true, NULL_TREE,
 				  /*before=*/true, GSI_SAME_STMT);
Index: gcc/testsuite/gcc.dg/tree-ssa/vrp78.c
===================================================================
--- /dev/null (new file)
+++ gcc/testsuite/gcc.dg/tree-ssa/vrp78.c (revision 0)
@@ -0,0 +1,34 @@ 
+/* { dg-do link } */
+/* { dg-options "-O2" } */
+
+/* Based on f3 from vrp63.c, but with switch instead of if-chain.  */
+
+extern void link_error (void);
+
+void
+f3 (int s)
+{
+  if (s >> 3 == -2)
+    /* s in range [ -16, -9].  */
+    ;
+  else
+    {
+      /* s in range ~[-16, -9], so none of the case labels can be taken.  */
+      switch (s)
+	{
+	case -16:
+	case -12:
+	case -9:
+	  link_error ();
+	  break;
+	default:
+	  break;
+	}
+    }
+}
+
+int
+main ()
+{
+  return 0;
+}