diff mbox series

gimple-low: Fix up handling of volatile automatic vars in assume attribute [PR110754]

Message ID ZcdF270qXqRGFzCz@tucnak
State New
Headers show
Series gimple-low: Fix up handling of volatile automatic vars in assume attribute [PR110754] | expand

Commit Message

Jakub Jelinek Feb. 10, 2024, 9:46 a.m. UTC 
Hi!

As the following testcases show, the gimple-low outlining of assume
magic functions handled volatile automatic vars (including
parameters/results) like non-volatile ones except it copied volatile
to the new PARM_DECL, which has the undesirable effect that a load
from the volatile var is passed to IFN_ASSUME and so there is a
side-effect there even when side-effects of the assume attribute
shouldn't be evaluated.

The following patch fixes that by passing address of the volatile
variables/parameters/results instead and doing loads or stores from it
or to it where it was originally accessed in the assume attribute
expression.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I guess we want to backport it to 13 eventually too.

2024-02-10  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/110754
	* gimple-low.cc (assumption_copy_decl): For TREE_THIS_VOLATILE
	decls create PARM_DECL with pointer to original type, set
	TREE_READONLY and keep TREE_THIS_VOLATILE, TREE_ADDRESSABLE,
	DECL_NOT_GIMPLE_REG_P and DECL_BY_REFERENCE cleared.
	(adjust_assumption_stmt_op): For remapped TREE_THIS_VOLATILE decls
	wrap PARM_DECL into a simple TREE_THIS_NO_TRAP MEM_REF.
	(lower_assumption): For TREE_THIS_VOLATILE vars pass ADDR_EXPR
	of the var as argument.

	* gcc.dg/attr-assume-6.c: New test.
	* g++.dg/cpp23/attr-assume12.C: New test.


	Jakub

Comments

Richard Biener Feb. 10, 2024, 9:48 a.m. UTC | #1 
> Am 10.02.2024 um 10:46 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> As the following testcases show, the gimple-low outlining of assume
> magic functions handled volatile automatic vars (including
> parameters/results) like non-volatile ones except it copied volatile
> to the new PARM_DECL, which has the undesirable effect that a load
> from the volatile var is passed to IFN_ASSUME and so there is a
> side-effect there even when side-effects of the assume attribute
> shouldn't be evaluated.
> 
> The following patch fixes that by passing address of the volatile
> variables/parameters/results instead and doing loads or stores from it
> or to it where it was originally accessed in the assume attribute
> expression.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> I guess we want to backport it to 13 eventually too.

Ok for trunk and branch after a while.

Richard 

> 2024-02-10  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR middle-end/110754
>    * gimple-low.cc (assumption_copy_decl): For TREE_THIS_VOLATILE
>    decls create PARM_DECL with pointer to original type, set
>    TREE_READONLY and keep TREE_THIS_VOLATILE, TREE_ADDRESSABLE,
>    DECL_NOT_GIMPLE_REG_P and DECL_BY_REFERENCE cleared.
>    (adjust_assumption_stmt_op): For remapped TREE_THIS_VOLATILE decls
>    wrap PARM_DECL into a simple TREE_THIS_NO_TRAP MEM_REF.
>    (lower_assumption): For TREE_THIS_VOLATILE vars pass ADDR_EXPR
>    of the var as argument.
> 
>    * gcc.dg/attr-assume-6.c: New test.
>    * g++.dg/cpp23/attr-assume12.C: New test.
> 
> --- gcc/gimple-low.cc.jj    2024-02-01 09:16:51.265388345 +0100
> +++ gcc/gimple-low.cc    2024-02-09 15:29:16.003289765 +0100
> @@ -374,15 +374,22 @@ assumption_copy_decl (tree decl, copy_bo
>   gcc_assert (VAR_P (decl)
>          || TREE_CODE (decl) == PARM_DECL
>          || TREE_CODE (decl) == RESULT_DECL);
> +  if (TREE_THIS_VOLATILE (decl))
> +    type = build_pointer_type (type);
>   tree copy = build_decl (DECL_SOURCE_LOCATION (decl),
>              PARM_DECL, DECL_NAME (decl), type);
>   if (DECL_PT_UID_SET_P (decl))
>     SET_DECL_PT_UID (copy, DECL_PT_UID (decl));
> -  TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl);
> -  TREE_READONLY (copy) = TREE_READONLY (decl);
> -  TREE_THIS_VOLATILE (copy) = TREE_THIS_VOLATILE (decl);
> -  DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl);
> -  DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl);
> +  TREE_THIS_VOLATILE (copy) = 0;
> +  if (TREE_THIS_VOLATILE (decl))
> +    TREE_READONLY (copy) = 1;
> +  else
> +    {
> +      TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl);
> +      TREE_READONLY (copy) = TREE_READONLY (decl);
> +      DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl);
> +      DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl);
> +    }
>   DECL_ARG_TYPE (copy) = type;
>   ((lower_assumption_data *) id)->decls.safe_push (decl);
>   return copy_decl_for_dup_finish (id, decl, copy);
> @@ -466,6 +473,11 @@ adjust_assumption_stmt_op (tree *tp, int
>     case PARM_DECL:
>     case RESULT_DECL:
>       *tp = remap_decl (t, &data->id);
> +      if (TREE_THIS_VOLATILE (t) && *tp != t)
> +    {
> +      *tp = build_simple_mem_ref (*tp);
> +      TREE_THIS_NOTRAP (*tp) = 1;
> +    }
>       break;
>     default:
>       break;
> @@ -600,6 +612,11 @@ lower_assumption (gimple_stmt_iterator *
>       /* Remaining arguments will be the variables/parameters
>     mentioned in the condition.  */
>       vargs[i - sz] = lad.decls[i - 1];
> +      if (TREE_THIS_VOLATILE (lad.decls[i - 1]))
> +    {
> +      TREE_ADDRESSABLE (lad.decls[i - 1]) = 1;
> +      vargs[i - sz] = build_fold_addr_expr (lad.decls[i - 1]);
> +    }
>       /* If they have gimple types, we might need to regimplify
>     them to make the IFN_ASSUME call valid.  */
>       if (is_gimple_reg_type (TREE_TYPE (vargs[i - sz]))
> --- gcc/testsuite/gcc.dg/attr-assume-6.c.jj    2024-02-09 15:32:49.384325442 +0100
> +++ gcc/testsuite/gcc.dg/attr-assume-6.c    2024-02-09 15:48:27.612288853 +0100
> @@ -0,0 +1,14 @@
> +/* PR middle-end/110754 */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c23 -O2 -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "={v} a" "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } */
> +
> +int
> +foo (int x)
> +{
> +  volatile int a = x;
> +  [[gnu::assume (x == (a & 0))]];
> +  return x;
> +}
> --- gcc/testsuite/g++.dg/cpp23/attr-assume12.C.jj    2024-02-09 15:51:21.100878070 +0100
> +++ gcc/testsuite/g++.dg/cpp23/attr-assume12.C    2024-02-09 15:51:51.863450599 +0100
> @@ -0,0 +1,14 @@
> +// PR middle-end/110754
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O2 -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } }
> +// { dg-final { scan-tree-dump-not "={v} a" "optimized" } }
> +// { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } }
> +
> +int
> +foo (int x)
> +{
> +  volatile int a = x;
> +  [[gnu::assume (x == (a & 0))]];
> +  return x;
> +}
> 
>    Jakub
>
diff mbox series

Patch

--- gcc/gimple-low.cc.jj	2024-02-01 09:16:51.265388345 +0100
+++ gcc/gimple-low.cc	2024-02-09 15:29:16.003289765 +0100
@@ -374,15 +374,22 @@  assumption_copy_decl (tree decl, copy_bo
   gcc_assert (VAR_P (decl)
 	      || TREE_CODE (decl) == PARM_DECL
 	      || TREE_CODE (decl) == RESULT_DECL);
+  if (TREE_THIS_VOLATILE (decl))
+    type = build_pointer_type (type);
   tree copy = build_decl (DECL_SOURCE_LOCATION (decl),
 			  PARM_DECL, DECL_NAME (decl), type);
   if (DECL_PT_UID_SET_P (decl))
     SET_DECL_PT_UID (copy, DECL_PT_UID (decl));
-  TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl);
-  TREE_READONLY (copy) = TREE_READONLY (decl);
-  TREE_THIS_VOLATILE (copy) = TREE_THIS_VOLATILE (decl);
-  DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl);
-  DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl);
+  TREE_THIS_VOLATILE (copy) = 0;
+  if (TREE_THIS_VOLATILE (decl))
+    TREE_READONLY (copy) = 1;
+  else
+    {
+      TREE_ADDRESSABLE (copy) = TREE_ADDRESSABLE (decl);
+      TREE_READONLY (copy) = TREE_READONLY (decl);
+      DECL_NOT_GIMPLE_REG_P (copy) = DECL_NOT_GIMPLE_REG_P (decl);
+      DECL_BY_REFERENCE (copy) = DECL_BY_REFERENCE (decl);
+    }
   DECL_ARG_TYPE (copy) = type;
   ((lower_assumption_data *) id)->decls.safe_push (decl);
   return copy_decl_for_dup_finish (id, decl, copy);
@@ -466,6 +473,11 @@  adjust_assumption_stmt_op (tree *tp, int
     case PARM_DECL:
     case RESULT_DECL:
       *tp = remap_decl (t, &data->id);
+      if (TREE_THIS_VOLATILE (t) && *tp != t)
+	{
+	  *tp = build_simple_mem_ref (*tp);
+	  TREE_THIS_NOTRAP (*tp) = 1;
+	}
       break;
     default:
       break;
@@ -600,6 +612,11 @@  lower_assumption (gimple_stmt_iterator *
       /* Remaining arguments will be the variables/parameters
 	 mentioned in the condition.  */
       vargs[i - sz] = lad.decls[i - 1];
+      if (TREE_THIS_VOLATILE (lad.decls[i - 1]))
+	{
+	  TREE_ADDRESSABLE (lad.decls[i - 1]) = 1;
+	  vargs[i - sz] = build_fold_addr_expr (lad.decls[i - 1]);
+	}
       /* If they have gimple types, we might need to regimplify
 	 them to make the IFN_ASSUME call valid.  */
       if (is_gimple_reg_type (TREE_TYPE (vargs[i - sz]))
--- gcc/testsuite/gcc.dg/attr-assume-6.c.jj	2024-02-09 15:32:49.384325442 +0100
+++ gcc/testsuite/gcc.dg/attr-assume-6.c	2024-02-09 15:48:27.612288853 +0100
@@ -0,0 +1,14 @@ 
+/* PR middle-end/110754 */
+/* { dg-do compile } */
+/* { dg-options "-std=c23 -O2 -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "={v} a" "optimized" } } */
+/* { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } } */
+
+int
+foo (int x)
+{
+  volatile int a = x;
+  [[gnu::assume (x == (a & 0))]];
+  return x;
+}
--- gcc/testsuite/g++.dg/cpp23/attr-assume12.C.jj	2024-02-09 15:51:21.100878070 +0100
+++ gcc/testsuite/g++.dg/cpp23/attr-assume12.C	2024-02-09 15:51:51.863450599 +0100
@@ -0,0 +1,14 @@ 
+// PR middle-end/110754
+// { dg-do compile { target c++11 } }
+// { dg-options "-O2 -fdump-tree-optimized" }
+// { dg-final { scan-tree-dump-times "a ={v} x" 1 "optimized" } }
+// { dg-final { scan-tree-dump-not "={v} a" "optimized" } }
+// { dg-final { scan-tree-dump-times "return 0;" 1 "optimized" } }
+
+int
+foo (int x)
+{
+  volatile int a = x;
+  [[gnu::assume (x == (a & 0))]];
+  return x;
+}