diff mbox series

ifcvt: Don't introduce trapping or faulting reads in noce_try_sign_mask [PR106032]

Message ID YrF/itA0wivs3iXh@tucnak
State New
Headers
Series ifcvt: Don't introduce trapping or faulting reads in noce_try_sign_mask [PR106032] |

Commit Message

Jakub Jelinek June 21, 2022, 8:21 a.m. UTC 
  Hi!

noce_try_sign_mask as documented will optimize
  if (c < 0)
    x = t;
  else
    x = 0;
into x = (c >> bitsm1) & t;
The optimization is done if either t is unconditional
(e.g. for
  x = t;
  if (c >= 0)
    x = 0;
) or if it is cheap.  We already check that t doesn't have side-effects,
but if t is conditional, we need to punt also if it may trap or fault,
as we make it unconditional.

I've briefly skimmed other noce_try* optimizations and didn't find one that
would suffer from the same problem.

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

2022-06-21  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/106032
	* ifcvt.cc (noce_try_sign_mask): Punt if !t_unconditional, and
	t may_trap_or_fault_p, even if it is cheap.

	* gcc.c-torture/execute/pr106032.c: New test.


	Jakub

Comments

Richard Biener June 21, 2022, 9:05 a.m. UTC | #1 
On Tue, 21 Jun 2022, Jakub Jelinek wrote:

> Hi!
> 
> noce_try_sign_mask as documented will optimize
>   if (c < 0)
>     x = t;
>   else
>     x = 0;
> into x = (c >> bitsm1) & t;
> The optimization is done if either t is unconditional
> (e.g. for
>   x = t;
>   if (c >= 0)
>     x = 0;
> ) or if it is cheap.  We already check that t doesn't have side-effects,
> but if t is conditional, we need to punt also if it may trap or fault,
> as we make it unconditional.
> 
> I've briefly skimmed other noce_try* optimizations and didn't find one that
> would suffer from the same problem.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Thanks,
Richard.

> 2022-06-21  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR rtl-optimization/106032
> 	* ifcvt.cc (noce_try_sign_mask): Punt if !t_unconditional, and
> 	t may_trap_or_fault_p, even if it is cheap.
> 
> 	* gcc.c-torture/execute/pr106032.c: New test.
> 
> --- gcc/ifcvt.cc.jj	2022-04-26 10:11:51.951558338 +0200
> +++ gcc/ifcvt.cc	2022-06-20 17:44:18.638394338 +0200
> @@ -2833,18 +2833,19 @@ noce_try_sign_mask (struct noce_if_info
>      return FALSE;
>  
>    /* This is only profitable if T is unconditionally executed/evaluated in the
> -     original insn sequence or T is cheap.  The former happens if B is the
> -     non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no
> -     INSN_B which can happen for e.g. conditional stores to memory.  For the
> -     cost computation use the block TEST_BB where the evaluation will end up
> -     after the transformation.  */
> +     original insn sequence or T is cheap and can't trap or fault.  The former
> +     happens if B is the non-zero (T) value and if INSN_B was taken from
> +     TEST_BB, or there was no INSN_B which can happen for e.g. conditional
> +     stores to memory.  For the cost computation use the block TEST_BB where
> +     the evaluation will end up after the transformation.  */
>    t_unconditional
>      = (t == if_info->b
>         && (if_info->insn_b == NULL_RTX
>  	   || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb));
>    if (!(t_unconditional
> -	|| (set_src_cost (t, mode, if_info->speed_p)
> -	    < COSTS_N_INSNS (2))))
> +	|| ((set_src_cost (t, mode, if_info->speed_p)
> +	     < COSTS_N_INSNS (2))
> +	    && !may_trap_or_fault_p (t))))
>      return FALSE;
>  
>    if (!noce_can_force_operand (t))
> --- gcc/testsuite/gcc.c-torture/execute/pr106032.c.jj	2022-06-20 18:00:01.064352904 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr106032.c	2022-06-20 17:59:41.714600349 +0200
> @@ -0,0 +1,21 @@
> +/* PR rtl-optimization/106032 */
> +
> +__attribute__((noipa)) int
> +foo (int x, int *y)
> +{
> +  int a = 0;
> +  if (x < 0)
> +    a = *y;
> +  return a;  
> +}
> +
> +int
> +main ()
> +{
> +  int a = 42;
> +  if (foo (0, 0) != 0 || foo (1, 0) != 0)
> +    __builtin_abort ();
> +  if (foo (-1, &a) != 42 || foo (-42, &a) != 42)
> +    __builtin_abort ();
> +  return 0;
> +}
> 
> 	Jakub
> 
>
diff mbox series

Patch

--- gcc/ifcvt.cc.jj	2022-04-26 10:11:51.951558338 +0200
+++ gcc/ifcvt.cc	2022-06-20 17:44:18.638394338 +0200
@@ -2833,18 +2833,19 @@  noce_try_sign_mask (struct noce_if_info
     return FALSE;
 
   /* This is only profitable if T is unconditionally executed/evaluated in the
-     original insn sequence or T is cheap.  The former happens if B is the
-     non-zero (T) value and if INSN_B was taken from TEST_BB, or there was no
-     INSN_B which can happen for e.g. conditional stores to memory.  For the
-     cost computation use the block TEST_BB where the evaluation will end up
-     after the transformation.  */
+     original insn sequence or T is cheap and can't trap or fault.  The former
+     happens if B is the non-zero (T) value and if INSN_B was taken from
+     TEST_BB, or there was no INSN_B which can happen for e.g. conditional
+     stores to memory.  For the cost computation use the block TEST_BB where
+     the evaluation will end up after the transformation.  */
   t_unconditional
     = (t == if_info->b
        && (if_info->insn_b == NULL_RTX
 	   || BLOCK_FOR_INSN (if_info->insn_b) == if_info->test_bb));
   if (!(t_unconditional
-	|| (set_src_cost (t, mode, if_info->speed_p)
-	    < COSTS_N_INSNS (2))))
+	|| ((set_src_cost (t, mode, if_info->speed_p)
+	     < COSTS_N_INSNS (2))
+	    && !may_trap_or_fault_p (t))))
     return FALSE;
 
   if (!noce_can_force_operand (t))
--- gcc/testsuite/gcc.c-torture/execute/pr106032.c.jj	2022-06-20 18:00:01.064352904 +0200
+++ gcc/testsuite/gcc.c-torture/execute/pr106032.c	2022-06-20 17:59:41.714600349 +0200
@@ -0,0 +1,21 @@ 
+/* PR rtl-optimization/106032 */
+
+__attribute__((noipa)) int
+foo (int x, int *y)
+{
+  int a = 0;
+  if (x < 0)
+    a = *y;
+  return a;  
+}
+
+int
+main ()
+{
+  int a = 42;
+  if (foo (0, 0) != 0 || foo (1, 0) != 0)
+    __builtin_abort ();
+  if (foo (-1, &a) != 42 || foo (-42, &a) != 42)
+    __builtin_abort ();
+  return 0;
+}