[C++ PATCH] Fix cp_fold dropping TREE_THIS_VOLATILE flag (PR c++/71372)

[C++ PATCH] Fix cp_fold dropping TREE_THIS_VOLATILE flag (PR c++/71372)

[Jakub Jelinek]

Hi!

When cp_fold is called on INDIRECT_REF and ARRAY*_REF and any of the
arguments change in the recursive calls, we fail to copy TREE_THIS_VOLATILE
flag.

Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk/6.2?

2016-06-02  Jakub Jelinek  <jakub@redhat.com>

	PR c++/71372
	* cp-gimplify.c (cp_fold): For INDIRECT_REF, if the folded expression
	is INDIRECT_REF or MEM_REF, copy over TREE_READONLY, TREE_SIDE_EFFECTS
	and TREE_THIS_VOLATILE flags.  For ARRAY_REF and ARRAY_RANGE_REF, copy
	over TREE_READONLY, TREE_SIDE_EFFECTS and TREE_THIS_VOLATILE flags
	to the newly built tree.

	* c-c++-common/pr71372.c: New test.

--- gcc/cp/cp-gimplify.c.jj	2016-05-26 10:38:01.000000000 +0200
+++ gcc/cp/cp-gimplify.c	2016-06-02 10:21:33.903655321 +0200
@@ -2035,7 +2035,16 @@ cp_fold (tree x)
 	  if (op0 == error_mark_node)
 	    x = error_mark_node;
 	  else
-	    x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
+	    {
+	      x = fold_build1_loc (loc, code, TREE_TYPE (x), op0);
+	      if (code == INDIRECT_REF
+		  && (INDIRECT_REF_P (x) || TREE_CODE (x) == MEM_REF))
+		{
+		  TREE_READONLY (x) = TREE_READONLY (org_x);
+		  TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
+		  TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
+		}
+	    }
 	}
       else
 	x = fold (x);
@@ -2312,7 +2321,12 @@ cp_fold (tree x)
 	      || op3 == error_mark_node)
 	    x = error_mark_node;
 	  else
-	    x = build4_loc (loc, code, TREE_TYPE (x), op0, op1, op2, op3);
+	    {
+	      x = build4_loc (loc, code, TREE_TYPE (x), op0, op1, op2, op3);
+	      TREE_READONLY (x) = TREE_READONLY (org_x);
+	      TREE_SIDE_EFFECTS (x) = TREE_SIDE_EFFECTS (org_x);
+	      TREE_THIS_VOLATILE (x) = TREE_THIS_VOLATILE (org_x);
+	    }
 	}
 
       x = fold (x);
--- gcc/testsuite/c-c++-common/pr71372.c.jj	2016-06-02 10:53:40.994678549 +0200
+++ gcc/testsuite/c-c++-common/pr71372.c	2016-06-02 10:53:04.000000000 +0200
@@ -0,0 +1,14 @@
+/* PR c++/71372 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized" } */
+
+void
+foo (volatile int *p, int q)
+{
+  *(volatile int *)p = 0;
+  *(p + (q - q) + 1) = 0;
+  *(p + (q - q) + 2) = 0;
+  *(p + (q - q) + 3) = 0;
+}
+
+/* { dg-final { scan-tree-dump-times " ={v} " 4 "optimized" } } */

	Jakub

Re: [C++ PATCH] Fix cp_fold dropping TREE_THIS_VOLATILE flag (PR c++/71372)

[Jason Merrill]

On Thu, Jun 2, 2016 at 8:13 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> When cp_fold is called on INDIRECT_REF and ARRAY*_REF and any of the
> arguments change in the recursive calls, we fail to copy TREE_THIS_VOLATILE
> flag.
>
>         PR c++/71372
>         * cp-gimplify.c (cp_fold): For INDIRECT_REF, if the folded expression
>         is INDIRECT_REF or MEM_REF, copy over TREE_READONLY, TREE_SIDE_EFFECTS
>         and TREE_THIS_VOLATILE flags.  For ARRAY_REF and ARRAY_RANGE_REF, copy
>         over TREE_READONLY, TREE_SIDE_EFFECTS and TREE_THIS_VOLATILE flags
>         to the newly built tree.

Hmm, maybe we should be using build_indirect_ref and build_array_ref.
Or perhaps factor the simple build and set flags out of
cp_build_indirect_ref and call that.

If you don't feel like making that change, the patch is also OK as is.

Jason

Re: [C++ PATCH] Fix cp_fold dropping TREE_THIS_VOLATILE flag (PR c++/71372)

[Jakub Jelinek]

On Thu, Jun 02, 2016 at 11:14:48AM -0400, Jason Merrill wrote:
> On Thu, Jun 2, 2016 at 8:13 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > When cp_fold is called on INDIRECT_REF and ARRAY*_REF and any of the
> > arguments change in the recursive calls, we fail to copy TREE_THIS_VOLATILE
> > flag.
> >
> >         PR c++/71372
> >         * cp-gimplify.c (cp_fold): For INDIRECT_REF, if the folded expression
> >         is INDIRECT_REF or MEM_REF, copy over TREE_READONLY, TREE_SIDE_EFFECTS
> >         and TREE_THIS_VOLATILE flags.  For ARRAY_REF and ARRAY_RANGE_REF, copy
> >         over TREE_READONLY, TREE_SIDE_EFFECTS and TREE_THIS_VOLATILE flags
> >         to the newly built tree.
> 
> Hmm, maybe we should be using build_indirect_ref and build_array_ref.

Those look too much front-endish to me (having lots of diagnostics etc. in them
for something that should have been already long diagnosed).

> Or perhaps factor the simple build and set flags out of
> cp_build_indirect_ref and call that.
> 
> If you don't feel like making that change, the patch is also OK as is.

I've tried to follow what the C FE does here:

      if (op0 != orig_op0
          && code == ADDR_EXPR
          && (op1 = get_base_address (op0)) != NULL_TREE
          && INDIRECT_REF_P (op1)
          && TREE_CONSTANT (TREE_OPERAND (op1, 0)))
        ret = fold_convert_loc (loc, TREE_TYPE (expr), fold_offsetof_1 (op0));
      else if (op0 != orig_op0 || in_init)
        ret = in_init
          ? fold_build1_initializer_loc (loc, code, TREE_TYPE (expr), op0)
          : fold_build1_loc (loc, code, TREE_TYPE (expr), op0);
      else
        ret = fold (expr);
      if (code == INDIRECT_REF
          && ret != expr
          && INDIRECT_REF_P (ret))
        {
          TREE_READONLY (ret) = TREE_READONLY (expr);
          TREE_SIDE_EFFECTS (ret) = TREE_SIDE_EFFECTS (expr);
          TREE_THIS_VOLATILE (ret) = TREE_THIS_VOLATILE (expr);
        }

and similar.

Note, we have also the same
problem in fold-const.c (fold), though we'd hit that more rarely than this, and
there we also should just copy the flags.

	Jakub