C PATCH for c/65345 (file-scope _Atomic expansion, this time with floats)

C PATCH for c/65345 (file-scope _Atomic expansion, this time with floats)

[Marek Polacek]

You were right: my earlier fix for c/65345 only handled non-float types.
This patch thus handles even the float types.  The fix is analogical to the
previous: use create_tmp_var_raw and TARGET_EXPR to avoid ICE.

This only fixes x86 though, other arches will need something similar.  You'll
notice that the tests are XFAILed on arches other than i?86/x86_64, hope that
is fine.

I have also tried
$ for i in 387 387+sse 387,sse both sse sse+387 sse,387; do ./cc1 -quiet pr65345-3.c -mfpmath=$i; done
and
$ for i in 387 387+sse 387,sse both sse sse+387 sse,387; do ./cc1 -quiet pr65345-4.c -mfpmath=$i; done
and that doesn't ICE.
Also .gimple dumps for c11-atomic-1.c with/without this patch look the same.

Bootstrapped/regtested on x86_64-linux, ok for trunk after the non-float part
is in?

2015-04-14  Marek Polacek  <polacek@redhat.com>

	PR c/65345
	* config/i386/i386.c (ix86_atomic_assign_expand_fenv): Adjust to use
	create_tmp_var_raw rather than create_tmp_var.

	* gcc.dg/atomic/pr65345-4.c: New test.
	* gcc.dg/pr65345-3.c: New test.

diff --git gcc/config/i386/i386.c gcc/config/i386/i386.c
index 3263656..23e9013 100644
--- gcc/config/i386/i386.c
+++ gcc/config/i386/i386.c
@@ -51646,13 +51646,13 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
 {
   if (!TARGET_80387 && !TARGET_SSE_MATH)
     return;
-  tree exceptions_var = create_tmp_var (integer_type_node);
+  tree exceptions_var = create_tmp_var_raw (integer_type_node);
   if (TARGET_80387)
     {
       tree fenv_index_type = build_index_type (size_int (6));
       tree fenv_type = build_array_type (unsigned_type_node, fenv_index_type);
-      tree fenv_var = create_tmp_var (fenv_type);
-      mark_addressable (fenv_var);
+      tree fenv_var = create_tmp_var_raw (fenv_type);
+      TREE_ADDRESSABLE (fenv_var) = 1;
       tree fenv_ptr = build_pointer_type (fenv_type);
       tree fenv_addr = build1 (ADDR_EXPR, fenv_ptr, fenv_var);
       fenv_addr = fold_convert (ptr_type_node, fenv_addr);
@@ -51662,10 +51662,12 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
       tree fnclex = ix86_builtins[IX86_BUILTIN_FNCLEX];
       tree hold_fnstenv = build_call_expr (fnstenv, 1, fenv_addr);
       tree hold_fnclex = build_call_expr (fnclex, 0);
-      *hold = build2 (COMPOUND_EXPR, void_type_node, hold_fnstenv,
+      fenv_var = build4 (TARGET_EXPR, fenv_type, fenv_var, hold_fnstenv,
+			 NULL_TREE, NULL_TREE);
+      *hold = build2 (COMPOUND_EXPR, void_type_node, fenv_var,
 		      hold_fnclex);
       *clear = build_call_expr (fnclex, 0);
-      tree sw_var = create_tmp_var (short_unsigned_type_node);
+      tree sw_var = create_tmp_var_raw (short_unsigned_type_node);
       tree fnstsw_call = build_call_expr (fnstsw, 0);
       tree sw_mod = build2 (MODIFY_EXPR, short_unsigned_type_node,
 			    sw_var, fnstsw_call);
@@ -51679,8 +51681,8 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
     }
   if (TARGET_SSE_MATH)
     {
-      tree mxcsr_orig_var = create_tmp_var (unsigned_type_node);
-      tree mxcsr_mod_var = create_tmp_var (unsigned_type_node);
+      tree mxcsr_orig_var = create_tmp_var_raw (unsigned_type_node);
+      tree mxcsr_mod_var = create_tmp_var_raw (unsigned_type_node);
       tree stmxcsr = ix86_builtins[IX86_BUILTIN_STMXCSR];
       tree ldmxcsr = ix86_builtins[IX86_BUILTIN_LDMXCSR];
       tree stmxcsr_hold_call = build_call_expr (stmxcsr, 0);
diff --git gcc/testsuite/gcc.dg/atomic/pr65345-4.c gcc/testsuite/gcc.dg/atomic/pr65345-4.c
index e69de29..5bd2ca3 100644
--- gcc/testsuite/gcc.dg/atomic/pr65345-4.c
+++ gcc/testsuite/gcc.dg/atomic/pr65345-4.c
@@ -0,0 +1,59 @@
+/* PR c/65345 */
+/* { dg-do run { xfail { ! "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "" } */
+
+#define CHECK(X) if (!(X)) __builtin_abort ()
+
+_Atomic float i = 5;
+_Atomic float j = 2;
+
+void
+fn1 (float a[(int) (i = 0)])
+{
+}
+
+void
+fn2 (float a[(int) (i += 2)])
+{
+}
+
+void
+fn3 (float a[(int) ++i])
+{
+}
+
+void
+fn4 (float a[(int) ++i])
+{
+}
+
+void
+fn5 (float a[(int) ++i][(int) (j = 10)])
+{
+}
+
+void
+fn6 (float a[(int) (i = 7)][(int) j--])
+{
+}
+
+int
+main ()
+{
+  float a[10];
+  float aa[10][10];
+  fn1 (a);
+  CHECK (i == 0);
+  fn2 (a);
+  CHECK (i == 2);
+  fn3 (a);
+  CHECK (i == 3);
+  fn4 (a);
+  CHECK (i == 4);
+  fn5 (aa);
+  CHECK (i == 5);
+  CHECK (j == 10);
+  fn6 (aa);
+  CHECK (i == 7);
+  CHECK (j == 9);
+}
diff --git gcc/testsuite/gcc.dg/pr65345-3.c gcc/testsuite/gcc.dg/pr65345-3.c
index e69de29..bd8088a 100644
--- gcc/testsuite/gcc.dg/pr65345-3.c
+++ gcc/testsuite/gcc.dg/pr65345-3.c
@@ -0,0 +1,36 @@
+/* PR c/65345 */
+/* { dg-do compile { xfail { ! "i?86-*-* x86_64-*-*" } } } */
+/* { dg-options "" } */
+
+_Atomic float i = 3.0f;
+
+float a1 = sizeof (i + 1.2);
+float a2 = sizeof (i = 0);
+float a3 = sizeof (i++);
+float a4 = sizeof (i--);
+float a5 = sizeof (-i);
+
+float b1 = _Alignof (i + 1);
+float b2 = _Alignof (i = 0);
+float b3 = _Alignof (i++);
+float b4 = _Alignof (i--);
+float b5 = _Alignof (-i);
+
+float c1 = i; /* { dg-error "initializer element is not constant" } */
+float c2 = (i ? 1 : 2); /* { dg-error "initializer element is not constant" } */
+float c3[(int) i]; /* { dg-error "variably modified" } */
+float c4 = 0 || i; /* { dg-error "initializer element is not constant" } */
+float c5 = (i += 10); /* { dg-error "initializer element is not constant" } */
+
+_Static_assert (_Generic (i, float: 1, default: 0) == 1, "1");
+_Static_assert (_Generic (i + 1, float: 1, default: 0) == 1, "2");
+_Static_assert (_Generic (i = 0, float: 1, default: 0) == 1, "3");
+_Static_assert (_Generic (i++, float: 1, default: 0) == 1, "4");
+_Static_assert (_Generic (i--, float: 1, default: 0) == 1, "5");
+
+_Atomic int sz = 2;
+void fn1 (float a[sz + 1]);
+void fn2 (float a[sz = 0]);
+void fn3 (float a[sz++]);
+void fn4 (float a[sz--]);
+void fn5 (float a[-sz]);

	Marek

Re: C PATCH for c/65345 (file-scope _Atomic expansion, this time with floats)

[Marek Polacek]

Ping.

On Tue, Apr 14, 2015 at 06:08:45PM +0200, Marek Polacek wrote:
> You were right: my earlier fix for c/65345 only handled non-float types.
> This patch thus handles even the float types.  The fix is analogical to the
> previous: use create_tmp_var_raw and TARGET_EXPR to avoid ICE.
> 
> This only fixes x86 though, other arches will need something similar.  You'll
> notice that the tests are XFAILed on arches other than i?86/x86_64, hope that
> is fine.
> 
> I have also tried
> $ for i in 387 387+sse 387,sse both sse sse+387 sse,387; do ./cc1 -quiet pr65345-3.c -mfpmath=$i; done
> and
> $ for i in 387 387+sse 387,sse both sse sse+387 sse,387; do ./cc1 -quiet pr65345-4.c -mfpmath=$i; done
> and that doesn't ICE.
> Also .gimple dumps for c11-atomic-1.c with/without this patch look the same.
> 
> Bootstrapped/regtested on x86_64-linux, ok for trunk after the non-float part
> is in?
> 
> 2015-04-14  Marek Polacek  <polacek@redhat.com>
> 
> 	PR c/65345
> 	* config/i386/i386.c (ix86_atomic_assign_expand_fenv): Adjust to use
> 	create_tmp_var_raw rather than create_tmp_var.
> 
> 	* gcc.dg/atomic/pr65345-4.c: New test.
> 	* gcc.dg/pr65345-3.c: New test.
> 
> diff --git gcc/config/i386/i386.c gcc/config/i386/i386.c
> index 3263656..23e9013 100644
> --- gcc/config/i386/i386.c
> +++ gcc/config/i386/i386.c
> @@ -51646,13 +51646,13 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
>  {
>    if (!TARGET_80387 && !TARGET_SSE_MATH)
>      return;
> -  tree exceptions_var = create_tmp_var (integer_type_node);
> +  tree exceptions_var = create_tmp_var_raw (integer_type_node);
>    if (TARGET_80387)
>      {
>        tree fenv_index_type = build_index_type (size_int (6));
>        tree fenv_type = build_array_type (unsigned_type_node, fenv_index_type);
> -      tree fenv_var = create_tmp_var (fenv_type);
> -      mark_addressable (fenv_var);
> +      tree fenv_var = create_tmp_var_raw (fenv_type);
> +      TREE_ADDRESSABLE (fenv_var) = 1;
>        tree fenv_ptr = build_pointer_type (fenv_type);
>        tree fenv_addr = build1 (ADDR_EXPR, fenv_ptr, fenv_var);
>        fenv_addr = fold_convert (ptr_type_node, fenv_addr);
> @@ -51662,10 +51662,12 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
>        tree fnclex = ix86_builtins[IX86_BUILTIN_FNCLEX];
>        tree hold_fnstenv = build_call_expr (fnstenv, 1, fenv_addr);
>        tree hold_fnclex = build_call_expr (fnclex, 0);
> -      *hold = build2 (COMPOUND_EXPR, void_type_node, hold_fnstenv,
> +      fenv_var = build4 (TARGET_EXPR, fenv_type, fenv_var, hold_fnstenv,
> +			 NULL_TREE, NULL_TREE);
> +      *hold = build2 (COMPOUND_EXPR, void_type_node, fenv_var,
>  		      hold_fnclex);
>        *clear = build_call_expr (fnclex, 0);
> -      tree sw_var = create_tmp_var (short_unsigned_type_node);
> +      tree sw_var = create_tmp_var_raw (short_unsigned_type_node);
>        tree fnstsw_call = build_call_expr (fnstsw, 0);
>        tree sw_mod = build2 (MODIFY_EXPR, short_unsigned_type_node,
>  			    sw_var, fnstsw_call);
> @@ -51679,8 +51681,8 @@ ix86_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
>      }
>    if (TARGET_SSE_MATH)
>      {
> -      tree mxcsr_orig_var = create_tmp_var (unsigned_type_node);
> -      tree mxcsr_mod_var = create_tmp_var (unsigned_type_node);
> +      tree mxcsr_orig_var = create_tmp_var_raw (unsigned_type_node);
> +      tree mxcsr_mod_var = create_tmp_var_raw (unsigned_type_node);
>        tree stmxcsr = ix86_builtins[IX86_BUILTIN_STMXCSR];
>        tree ldmxcsr = ix86_builtins[IX86_BUILTIN_LDMXCSR];
>        tree stmxcsr_hold_call = build_call_expr (stmxcsr, 0);
> diff --git gcc/testsuite/gcc.dg/atomic/pr65345-4.c gcc/testsuite/gcc.dg/atomic/pr65345-4.c
> index e69de29..5bd2ca3 100644
> --- gcc/testsuite/gcc.dg/atomic/pr65345-4.c
> +++ gcc/testsuite/gcc.dg/atomic/pr65345-4.c
> @@ -0,0 +1,59 @@
> +/* PR c/65345 */
> +/* { dg-do run { xfail { ! "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "" } */
> +
> +#define CHECK(X) if (!(X)) __builtin_abort ()
> +
> +_Atomic float i = 5;
> +_Atomic float j = 2;
> +
> +void
> +fn1 (float a[(int) (i = 0)])
> +{
> +}
> +
> +void
> +fn2 (float a[(int) (i += 2)])
> +{
> +}
> +
> +void
> +fn3 (float a[(int) ++i])
> +{
> +}
> +
> +void
> +fn4 (float a[(int) ++i])
> +{
> +}
> +
> +void
> +fn5 (float a[(int) ++i][(int) (j = 10)])
> +{
> +}
> +
> +void
> +fn6 (float a[(int) (i = 7)][(int) j--])
> +{
> +}
> +
> +int
> +main ()
> +{
> +  float a[10];
> +  float aa[10][10];
> +  fn1 (a);
> +  CHECK (i == 0);
> +  fn2 (a);
> +  CHECK (i == 2);
> +  fn3 (a);
> +  CHECK (i == 3);
> +  fn4 (a);
> +  CHECK (i == 4);
> +  fn5 (aa);
> +  CHECK (i == 5);
> +  CHECK (j == 10);
> +  fn6 (aa);
> +  CHECK (i == 7);
> +  CHECK (j == 9);
> +}
> diff --git gcc/testsuite/gcc.dg/pr65345-3.c gcc/testsuite/gcc.dg/pr65345-3.c
> index e69de29..bd8088a 100644
> --- gcc/testsuite/gcc.dg/pr65345-3.c
> +++ gcc/testsuite/gcc.dg/pr65345-3.c
> @@ -0,0 +1,36 @@
> +/* PR c/65345 */
> +/* { dg-do compile { xfail { ! "i?86-*-* x86_64-*-*" } } } */
> +/* { dg-options "" } */
> +
> +_Atomic float i = 3.0f;
> +
> +float a1 = sizeof (i + 1.2);
> +float a2 = sizeof (i = 0);
> +float a3 = sizeof (i++);
> +float a4 = sizeof (i--);
> +float a5 = sizeof (-i);
> +
> +float b1 = _Alignof (i + 1);
> +float b2 = _Alignof (i = 0);
> +float b3 = _Alignof (i++);
> +float b4 = _Alignof (i--);
> +float b5 = _Alignof (-i);
> +
> +float c1 = i; /* { dg-error "initializer element is not constant" } */
> +float c2 = (i ? 1 : 2); /* { dg-error "initializer element is not constant" } */
> +float c3[(int) i]; /* { dg-error "variably modified" } */
> +float c4 = 0 || i; /* { dg-error "initializer element is not constant" } */
> +float c5 = (i += 10); /* { dg-error "initializer element is not constant" } */
> +
> +_Static_assert (_Generic (i, float: 1, default: 0) == 1, "1");
> +_Static_assert (_Generic (i + 1, float: 1, default: 0) == 1, "2");
> +_Static_assert (_Generic (i = 0, float: 1, default: 0) == 1, "3");
> +_Static_assert (_Generic (i++, float: 1, default: 0) == 1, "4");
> +_Static_assert (_Generic (i--, float: 1, default: 0) == 1, "5");
> +
> +_Atomic int sz = 2;
> +void fn1 (float a[sz + 1]);
> +void fn2 (float a[sz = 0]);
> +void fn3 (float a[sz++]);
> +void fn4 (float a[sz--]);
> +void fn5 (float a[-sz]);

	Marek

Re: C PATCH for c/65345 (file-scope _Atomic expansion, this time with floats)

[Joseph Myers]

On Tue, 14 Apr 2015, Marek Polacek wrote:

> You were right: my earlier fix for c/65345 only handled non-float types.
> This patch thus handles even the float types.  The fix is analogical to the
> previous: use create_tmp_var_raw and TARGET_EXPR to avoid ICE.
> 
> This only fixes x86 though, other arches will need something similar.  You'll
> notice that the tests are XFAILed on arches other than i?86/x86_64, hope that
> is fine.

I don't think default XFAILing is appropriate; like c11-atomic-exec-5.c, I 
think the failures should be visible to show that there is something 
target-specific to be done (and then target maintainers can choose to 
XFAIL with reference to a bug filed in Bugzilla if they so wish, but the 
default should be non-XFAILed).  I also think you should identify the 
targets that look like they would be affected (based on source code 
inspection) and inform the relevant target maintainers of the issue and of 
the appropriate approach for a fix.

Apart from the XFAILing, the patch is OK in the absence of x86 back-end 
maintainer objections within 48 hours.

-- 
Joseph S. Myers
joseph@codesourcery.com