public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C PATCH RFC] Drop qualifiers during lvalue conversion
@ 2020-11-07 15:17 Uecker, Martin
  2020-11-09 23:41 ` Joseph Myers
  0 siblings, 1 reply; 10+ messages in thread
From: Uecker, Martin @ 2020-11-07 15:17 UTC (permalink / raw)
  To: gcc-patches; +Cc: joseph


To better understand what impact this may have, I added code
to drop the qualifiers in 'convert_lvalue_to_rvalue'. Here
is the patch.

There are three new errors in the testsuite:

In 'gcc.dg/cond-constqual-1.c' we test for the opposite
behavior for conditional operators. I do not know why.
We could just invert the test.

In 'gcc.dg/pr60195.c' there is a new warning for a statement
with no effect for an unused atomic load. This warning
seems correct to me and I think it is wrong that we do not
currently have it.


Finally, there is a failure:

FAIL: gcc.dg/tree-ssa/pr33723.c scan-tree-dump-not gimple "t = D"

This is a partial regression related to

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=33723

where we now produce a temporary if there is a
qualifier:

void
baz1 (void)
{
  T t;
  t = (const T) { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } };
  test (&t);
}

Not sure what to do about it, maybe 'convert' is not
the right function to use.

Best,
Martin



diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 96840377d90..aeacd30badd 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2080,6 +2080,8 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
     exp = default_function_array_conversion (loc, exp);
   if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
     exp.value = require_complete_type (loc, exp.value);
+  if (convert_p && !error_operand_p (exp.value))
+    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
exp.value);
   if (really_atomic_lvalue (exp.value))
     {
       vec<tree, va_gc> *params;
diff --git a/gcc/testsuite/gcc.dg/cond-constqual-1.c b/gcc/testsuite/gcc.dg/cond-constqual-1.c
index 3354c7214a4..b5a09cb0038 100644
--- a/gcc/testsuite/gcc.dg/cond-constqual-1.c
+++ b/gcc/testsuite/gcc.dg/cond-constqual-1.c
@@ -11,5 +11,5 @@ test (void)
   __typeof__ (1 ? foo (0) : 0) texpr;
   __typeof__ (1 ? i : 0) texpr2;
   texpr = 0;  /* { dg-bogus "read-only variable" "conditional expression with call to const
function" } */
-  texpr2 = 0; /* { dg-error "read-only variable" "conditional expression with const variable" } */
+  texpr2 = 0; /* { dg-bogus "read-only variable" "conditional expression with const variable" } */
 }
diff --git a/gcc/testsuite/gcc.dg/lvalue-11.c b/gcc/testsuite/gcc.dg/lvalue-11.c
new file mode 100644
index 00000000000..45a97d86890
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lvalue-11.c
@@ -0,0 +1,46 @@
+/* test that lvalue conversions drops qualifiers, Bug 97702 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+
+void f(void)
+{
+ const int j;
+ typeof((0,j)) i10; i10 = j;;
+ typeof(+j) i11; i11 = j;;
+ typeof(-j) i12; i12 = j;;
+ typeof(1?j:0) i13; i13 = j;;
+ typeof((int)j) i14; i14 = j;;
+ typeof((const int)j) i15; i15 = j;;
+}
+
+void g(void)
+{
+ volatile int j;
+ typeof((0,j)) i21; i21 = j;;
+ typeof(+j) i22; i22 = j;;
+ typeof(-j) i23; i23 = j;;
+ typeof(1?j:0) i24; i24 = j;;
+ typeof((int)j) i25; i25 = j;;
+ typeof((volatile int)j) i26; i26 = j;;
+}
+
+void h(void)
+{
+ _Atomic int j;
+ typeof((0,j)) i32; i32 = j;;
+ typeof(+j) i33; i33 = j;;
+ typeof(-j) i34; i34 = j;;
+ typeof(1?j:0) i35; i35 = j;;
+ typeof((int)j) i36; i36 = j;;
+ typeof((_Atomic int)j) i37; i37 = j;;
+}
+
+void e(void)
+{
+ int* restrict j;
+ typeof((0,j)) i43; i43 = j;;
+ typeof(1?j:0) i44; i44 = j;;
+ typeof((int*)j) i45; i45 = j;;
+ typeof((int* restrict)j) i46; i46 = j;;
+}
diff --git a/gcc/testsuite/gcc.dg/pr60195.c b/gcc/testsuite/gcc.dg/pr60195.c
index 0a50a30be25..8eccf7f63ad 100644
--- a/gcc/testsuite/gcc.dg/pr60195.c
+++ b/gcc/testsuite/gcc.dg/pr60195.c
@@ -15,7 +15,7 @@ atomic_int
 fn2 (void)
 {
   atomic_int y = 0;
-  y;
+  y;		/* { dg-warning "statement with no effect" } */
   return y;
 }
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH RFC] Drop qualifiers during lvalue conversion
  2020-11-07 15:17 [C PATCH RFC] Drop qualifiers during lvalue conversion Uecker, Martin
@ 2020-11-09 23:41 ` Joseph Myers
  2020-11-15 20:26   ` Uecker, Martin
  0 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2020-11-09 23:41 UTC (permalink / raw)
  To: Uecker, Martin; +Cc: gcc-patches

On Sat, 7 Nov 2020, Uecker, Martin wrote:

> In 'gcc.dg/cond-constqual-1.c' we test for the opposite
> behavior for conditional operators. I do not know why.
> We could just invert the test.

That's probably a relic of the old idea that rvalues might actually have 
qualified type in some cases; it seems reasonable to invert the test.

>   t = (const T) { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } };
>   test (&t);
> }
> 
> Not sure what to do about it, maybe 'convert' is not
> the right function to use.

I think 'convert' is fine, but new code is probably needed in whatever 
implements the optimization for assignment from compound literals so that 
it works when there is a conversion that only changes qualifiers.

> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 96840377d90..aeacd30badd 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -2080,6 +2080,8 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
>      exp = default_function_array_conversion (loc, exp);
>    if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
>      exp.value = require_complete_type (loc, exp.value);
> +  if (convert_p && !error_operand_p (exp.value))
> +    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
> exp.value);

I think it might be safest to avoid doing any conversion in the case where 
the value is still of array type at this point (C90 non-lvalue arrays).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH RFC] Drop qualifiers during lvalue conversion
  2020-11-09 23:41 ` Joseph Myers
@ 2020-11-15 20:26   ` Uecker, Martin
  2020-11-16 22:50     ` Joseph Myers
  0 siblings, 1 reply; 10+ messages in thread
From: Uecker, Martin @ 2020-11-15 20:26 UTC (permalink / raw)
  To: joseph; +Cc: gcc-patches

Am Montag, den 09.11.2020, 23:41 +0000 schrieb Joseph Myers:
> On Sat, 7 Nov 2020, Uecker, Martin wrote:

> >   t = (const T) { { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 } };
> >   test (&t);
> > }
> > 
> > Not sure what to do about it, maybe 'convert' is not
> > the right function to use.
> 
> I think 'convert' is fine, but new code is probably needed in whatever 
> implements the optimization for assignment from compound literals so that 
> it works when there is a conversion that only changes qualifiers.

This seems to work now.

> > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > index 96840377d90..aeacd30badd 100644
> > --- a/gcc/c/c-typeck.c
> > +++ b/gcc/c/c-typeck.c
> > @@ -2080,6 +2080,8 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
> >      exp = default_function_array_conversion (loc, exp);
> >    if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
> >      exp.value = require_complete_type (loc, exp.value);
> > +  if (convert_p && !error_operand_p (exp.value))
> > +    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
> > exp.value);
> 
> I think it might be safest to avoid doing any conversion in the case where 
> the value is still of array type at this point (C90 non-lvalue arrays).

I added a test for arrays, but I am not sure what effect it has.
What would be C90 non-lvalue arrays?  Anything I can test?


diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 413109c916c..286f3d9cd6c 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2080,6 +2080,9 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
     exp = default_function_array_conversion (loc, exp);
   if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
     exp.value = require_complete_type (loc, exp.value);
+  if (convert_p && !error_operand_p (exp.value)
+      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
+    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED), exp.value);
   if (really_atomic_lvalue (exp.value))
     {
       vec<tree, va_gc> *params;
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 2566ec7f0af..f83b161b2e8 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5518,6 +5518,18 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
 	    return GS_OK;
 	  }
 
+	case NOP_EXPR:
+	  /* Pull out compound literal expressions from a NOP_EXPR.
+	     Those are created in the C FE to drop qualifiers during
+	     lvalue conversion.  */
+	  if (TREE_CODE (TREE_OPERAND (*from_p, 0)) == COMPOUND_LITERAL_EXPR)
+	    {
+	      *from_p = TREE_OPERAND (*from_p, 0);
+	      ret = GS_OK;
+	      changed = true;
+	    }
+	  break;
+
 	case COMPOUND_LITERAL_EXPR:
 	  {
 	    tree complit = TREE_OPERAND (*expr_p, 1);
diff --git a/gcc/testsuite/gcc.dg/cond-constqual-1.c b/gcc/testsuite/gcc.dg/cond-constqual-1.c
index 3354c7214a4..b5a09cb0038 100644
--- a/gcc/testsuite/gcc.dg/cond-constqual-1.c
+++ b/gcc/testsuite/gcc.dg/cond-constqual-1.c
@@ -11,5 +11,5 @@ test (void)
   __typeof__ (1 ? foo (0) : 0) texpr;
   __typeof__ (1 ? i : 0) texpr2;
   texpr = 0;  /* { dg-bogus "read-only variable" "conditional expression with call to const function" } */
-  texpr2 = 0; /* { dg-error "read-only variable" "conditional expression with const variable" } */
+  texpr2 = 0; /* { dg-bogus "read-only variable" "conditional expression with const variable" } */
 }
diff --git a/gcc/testsuite/gcc.dg/lvalue-11.c b/gcc/testsuite/gcc.dg/lvalue-11.c
new file mode 100644
index 00000000000..45a97d86890
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lvalue-11.c
@@ -0,0 +1,46 @@
+/* test that lvalue conversions drops qualifiers, Bug 97702 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+
+void f(void)
+{
+ const int j;
+ typeof((0,j)) i10; i10 = j;;
+ typeof(+j) i11; i11 = j;;
+ typeof(-j) i12; i12 = j;;
+ typeof(1?j:0) i13; i13 = j;;
+ typeof((int)j) i14; i14 = j;;
+ typeof((const int)j) i15; i15 = j;;
+}
+
+void g(void)
+{
+ volatile int j;
+ typeof((0,j)) i21; i21 = j;;
+ typeof(+j) i22; i22 = j;;
+ typeof(-j) i23; i23 = j;;
+ typeof(1?j:0) i24; i24 = j;;
+ typeof((int)j) i25; i25 = j;;
+ typeof((volatile int)j) i26; i26 = j;;
+}
+
+void h(void)
+{
+ _Atomic int j;
+ typeof((0,j)) i32; i32 = j;;
+ typeof(+j) i33; i33 = j;;
+ typeof(-j) i34; i34 = j;;
+ typeof(1?j:0) i35; i35 = j;;
+ typeof((int)j) i36; i36 = j;;
+ typeof((_Atomic int)j) i37; i37 = j;;
+}
+
+void e(void)
+{
+ int* restrict j;
+ typeof((0,j)) i43; i43 = j;;
+ typeof(1?j:0) i44; i44 = j;;
+ typeof((int*)j) i45; i45 = j;;
+ typeof((int* restrict)j) i46; i46 = j;;
+}
diff --git a/gcc/testsuite/gcc.dg/pr60195.c b/gcc/testsuite/gcc.dg/pr60195.c
index 0a50a30be25..8eccf7f63ad 100644
--- a/gcc/testsuite/gcc.dg/pr60195.c
+++ b/gcc/testsuite/gcc.dg/pr60195.c
@@ -15,7 +15,7 @@ atomic_int
 fn2 (void)
 {
   atomic_int y = 0;
-  y;
+  y;		/* { dg-warning "statement with no effect" } */
   return y;
 }
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH RFC] Drop qualifiers during lvalue conversion
  2020-11-15 20:26   ` Uecker, Martin
@ 2020-11-16 22:50     ` Joseph Myers
  2020-11-19  6:34       ` [C PATCH] " Uecker, Martin
  0 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2020-11-16 22:50 UTC (permalink / raw)
  To: Uecker, Martin; +Cc: gcc-patches

On Sun, 15 Nov 2020, Uecker, Martin wrote:

> > I think it might be safest to avoid doing any conversion in the case where 
> > the value is still of array type at this point (C90 non-lvalue arrays).
> 
> I added a test for arrays, but I am not sure what effect it has.
> What would be C90 non-lvalue arrays?  Anything I can test?

In C90, a non-lvalue array (an array member of a non-lvalue structure or 
union, obtained by a function return / assignment / conditional expression 
/ comma expression of structure or union type) did not get converted to a 
pointer.  This meant there was nothing much useful that could be done with 
such arrays, because you couldn't access their values (technically I think 
you could pass them in variable arguments and retrieve them in the called 
function with va_arg, but that doesn't help because it just gives another 
non-lvalue copy of the value that you still can't do anything useful 
with).

The possible breakage I'm concerned about isn't something that can readily 
be tested for; it's that if you create an unqualified array type directly 
with build_qualified_type (not going through c_build_qualified_type), you 
may break the invariants regarding how arrays of qualified element types 
are represented internally in GCC, and so cause subtle problems in code 
relying on such invariants.  (Cf. the problem with inconsistency in that 
area that I fixed with 
<https://gcc.gnu.org/legacy-ml/gcc-patches/2005-01/msg02180.html> (commit 
46df282378908dff9219749cd4cd576c155b2971).)  Avoiding these conversions in 
the case of arrays, as this version of the patch does, seems the simplest 
way to avoid any such issues arising.

> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 2566ec7f0af..f83b161b2e8 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -5518,6 +5518,18 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
>  	    return GS_OK;
>  	  }
>  
> +	case NOP_EXPR:
> +	  /* Pull out compound literal expressions from a NOP_EXPR.
> +	     Those are created in the C FE to drop qualifiers during
> +	     lvalue conversion.  */
> +	  if (TREE_CODE (TREE_OPERAND (*from_p, 0)) == COMPOUND_LITERAL_EXPR)

A NOP_EXPR might sometimes be doing an actual conversion (say the compound 
literal is (int){ INT_MAX }, converted to short).  I think this should 
check tree_ssa_useless_type_conversion to make sure it's safe to remove 
the conversion.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-16 22:50     ` Joseph Myers
@ 2020-11-19  6:34       ` Uecker, Martin
  2020-11-19 18:58         ` Joseph Myers
  2020-11-23 13:55         ` Christophe Lyon
  0 siblings, 2 replies; 10+ messages in thread
From: Uecker, Martin @ 2020-11-19  6:34 UTC (permalink / raw)
  To: joseph; +Cc: gcc-patches



Here is another version of the patch. The
only difference is the additional the check 
using 'tree_ssa_useless_type_conversion'.


Best,
Martin




    C: Drop qualifiers during lvalue conversion. PR97702
    
    2020-11-XX  Martin Uecker  <muecker@gwdg.de>

    gcc/
	    * gcc/gimplify.c (gimplify_modify_expr_rhs): Optimizie
	    NOP_EXPRs that contain compound literals.

    gcc/c/
            * c-typeck.c (convert_lvalue_to_rvalue): Drop qualifiers.
 
    gcc/testsuite/
	    * gcc.dg/cond-constqual-1.c: Adapt test.
	    * gcc.dg/lvalue-11.c: New test.
	    * gcc.dg/pr60195.c: Add warning.





diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 413109c916c..286f3d9cd6c 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -2080,6 +2080,9 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
     exp = default_function_array_conversion (loc, exp);
   if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
     exp.value = require_complete_type (loc, exp.value);
+  if (convert_p && !error_operand_p (exp.value)
+      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
+    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
exp.value);
   if (really_atomic_lvalue (exp.value))
     {
       vec<tree, va_gc> *params;
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 2566ec7f0af..fd0b5202b45 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -5518,6 +5518,19 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
 	    return GS_OK;
 	  }
 
+	case NOP_EXPR:
+	  /* Pull out compound literal expressions from a NOP_EXPR.
+	     Those are created in the C FE to drop qualifiers during
+	     lvalue conversion.  */
+	  if ((TREE_CODE (TREE_OPERAND (*from_p, 0)) == COMPOUND_LITERAL_EXPR)
+	      && tree_ssa_useless_type_conversion (*from_p))
+	    {
+	      *from_p = TREE_OPERAND (*from_p, 0);
+	      ret = GS_OK;
+	      changed = true;
+	    }
+	  break;
+
 	case COMPOUND_LITERAL_EXPR:
 	  {
 	    tree complit = TREE_OPERAND (*expr_p, 1);
diff --git a/gcc/testsuite/gcc.dg/cond-constqual-1.c b/gcc/testsuite/gcc.dg/cond-constqual-1.c
index 3354c7214a4..b5a09cb0038 100644
--- a/gcc/testsuite/gcc.dg/cond-constqual-1.c
+++ b/gcc/testsuite/gcc.dg/cond-constqual-1.c
@@ -11,5 +11,5 @@ test (void)
   __typeof__ (1 ? foo (0) : 0) texpr;
   __typeof__ (1 ? i : 0) texpr2;
   texpr = 0;  /* { dg-bogus "read-only variable" "conditional expression with call to const
function" } */
-  texpr2 = 0; /* { dg-error "read-only variable" "conditional expression with const variable" } */
+  texpr2 = 0; /* { dg-bogus "read-only variable" "conditional expression with const variable" } */
 }
diff --git a/gcc/testsuite/gcc.dg/lvalue-11.c b/gcc/testsuite/gcc.dg/lvalue-11.c
new file mode 100644
index 00000000000..45a97d86890
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/lvalue-11.c
@@ -0,0 +1,46 @@
+/* test that lvalue conversions drops qualifiers, Bug 97702 */
+/* { dg-do compile } */
+/* { dg-options "" } */
+
+
+void f(void)
+{
+ const int j;
+ typeof((0,j)) i10; i10 = j;;
+ typeof(+j) i11; i11 = j;;
+ typeof(-j) i12; i12 = j;;
+ typeof(1?j:0) i13; i13 = j;;
+ typeof((int)j) i14; i14 = j;;
+ typeof((const int)j) i15; i15 = j;;
+}
+
+void g(void)
+{
+ volatile int j;
+ typeof((0,j)) i21; i21 = j;;
+ typeof(+j) i22; i22 = j;;
+ typeof(-j) i23; i23 = j;;
+ typeof(1?j:0) i24; i24 = j;;
+ typeof((int)j) i25; i25 = j;;
+ typeof((volatile int)j) i26; i26 = j;;
+}
+
+void h(void)
+{
+ _Atomic int j;
+ typeof((0,j)) i32; i32 = j;;
+ typeof(+j) i33; i33 = j;;
+ typeof(-j) i34; i34 = j;;
+ typeof(1?j:0) i35; i35 = j;;
+ typeof((int)j) i36; i36 = j;;
+ typeof((_Atomic int)j) i37; i37 = j;;
+}
+
+void e(void)
+{
+ int* restrict j;
+ typeof((0,j)) i43; i43 = j;;
+ typeof(1?j:0) i44; i44 = j;;
+ typeof((int*)j) i45; i45 = j;;
+ typeof((int* restrict)j) i46; i46 = j;;
+}
diff --git a/gcc/testsuite/gcc.dg/pr60195.c b/gcc/testsuite/gcc.dg/pr60195.c
index 0a50a30be25..8eccf7f63ad 100644
--- a/gcc/testsuite/gcc.dg/pr60195.c
+++ b/gcc/testsuite/gcc.dg/pr60195.c
@@ -15,7 +15,7 @@ atomic_int
 fn2 (void)
 {
   atomic_int y = 0;
-  y;
+  y;		/* { dg-warning "statement with no effect" } */
   return y;
 }
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-19  6:34       ` [C PATCH] " Uecker, Martin
@ 2020-11-19 18:58         ` Joseph Myers
  2020-11-19 19:34           ` Uecker, Martin
  2020-11-23 13:55         ` Christophe Lyon
  1 sibling, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2020-11-19 18:58 UTC (permalink / raw)
  To: Uecker, Martin; +Cc: gcc-patches

On Thu, 19 Nov 2020, Uecker, Martin wrote:

> Here is another version of the patch. The
> only difference is the additional the check 
> using 'tree_ssa_useless_type_conversion'.

The code changes in this one are OK.  However, in the test:

> +void f(void)
> +{
> + const int j;
> + typeof((0,j)) i10; i10 = j;;
> + typeof(+j) i11; i11 = j;;
> + typeof(-j) i12; i12 = j;;
> + typeof(1?j:0) i13; i13 = j;;
> + typeof((int)j) i14; i14 = j;;
> + typeof((const int)j) i15; i15 = j;;
> +}

This test function seems fine.

> +void g(void)
> +{
> + volatile int j;
> + typeof((0,j)) i21; i21 = j;;
> + typeof(+j) i22; i22 = j;;
> + typeof(-j) i23; i23 = j;;
> + typeof(1?j:0) i24; i24 = j;;
> + typeof((int)j) i25; i25 = j;;
> + typeof((volatile int)j) i26; i26 = j;;
> +}
> +
> +void h(void)
> +{
> + _Atomic int j;
> + typeof((0,j)) i32; i32 = j;;
> + typeof(+j) i33; i33 = j;;
> + typeof(-j) i34; i34 = j;;
> + typeof(1?j:0) i35; i35 = j;;
> + typeof((int)j) i36; i36 = j;;
> + typeof((_Atomic int)j) i37; i37 = j;;
> +}
> +
> +void e(void)
> +{
> + int* restrict j;
> + typeof((0,j)) i43; i43 = j;;
> + typeof(1?j:0) i44; i44 = j;;
> + typeof((int*)j) i45; i45 = j;;
> + typeof((int* restrict)j) i46; i46 = j;;
> +}

But these tests don't look like they do anything useful (i.e. verify that 
typeof loses the qualifier), because testing by assignment like that only 
works with const.  You could do e.g.

volatile int j;
extern int i;
extern typeof((0,j)) i;

instead to verify the qualifier is removed.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-19 18:58         ` Joseph Myers
@ 2020-11-19 19:34           ` Uecker, Martin
  2020-11-19 20:53             ` Joseph Myers
  0 siblings, 1 reply; 10+ messages in thread
From: Uecker, Martin @ 2020-11-19 19:34 UTC (permalink / raw)
  To: joseph; +Cc: gcc-patches

Am Donnerstag, den 19.11.2020, 18:58 +0000 schrieb Joseph Myers:
> On Thu, 19 Nov 2020, Uecker, Martin wrote:

...
> 
> > +void g(void)
> > +{
> > + volatile int j;
> > + typeof((0,j)) i21; i21 = j;;
> > + typeof(+j) i22; i22 = j;;
> > + typeof(-j) i23; i23 = j;;
> > + typeof(1?j:0) i24; i24 = j;;
> > + typeof((int)j) i25; i25 = j;;
> > + typeof((volatile int)j) i26; i26 = j;;
> > +}
> > +
> > +void h(void)
> > +{
> > + _Atomic int j;
> > + typeof((0,j)) i32; i32 = j;;
> > + typeof(+j) i33; i33 = j;;
> > + typeof(-j) i34; i34 = j;;
> > + typeof(1?j:0) i35; i35 = j;;
> > + typeof((int)j) i36; i36 = j;;
> > + typeof((_Atomic int)j) i37; i37 = j;;
> > +}
> > +
> > +void e(void)
> > +{
> > + int* restrict j;
> > + typeof((0,j)) i43; i43 = j;;
> > + typeof(1?j:0) i44; i44 = j;;
> > + typeof((int*)j) i45; i45 = j;;
> > + typeof((int* restrict)j) i46; i46 = j;;
> > +}
> 
> But these tests don't look like they do anything useful (i.e. verify that 
> typeof loses the qualifier), because testing by assignment like that only 
> works with const.  You could do e.g.
> 
> volatile int j;
> extern int i;
> extern typeof((0,j)) i;
> 
> instead to verify the qualifier is removed.

Apparently I did not have enough coffee when
generalizing this to the other qualifiers. 

Ok, with the following test?



/* test that lvalue conversions drops qualifiers, Bug 97702 */
/* { dg-do compile } */
/* { dg-options "" } */


const int jc;
extern int j;
extern typeof(0,jc) j;
extern typeof(+jc) j;
extern typeof(-jc) j;
extern typeof(1?jc:0) j;
extern typeof((int)jc) j;
extern typeof((const int)jc) j;

volatile int kv;
extern int k;
extern typeof(0,kv) k;
extern typeof(+kv) k;
extern typeof(-kv) k;
extern typeof(1?kv:0) k;
extern typeof((int)kv) k;
extern typeof((volatile int)kv) k;

_Atomic int la;
extern int l;
extern typeof(0,la) l;
extern typeof(+la) l;
extern typeof(-la) l;
extern typeof(1?la:0) l;
extern typeof((int)la) l;
extern typeof((_Atomic int)la) l;

int * restrict mr;
extern int *m;
extern typeof(0,mr) m;
extern typeof(1?mr:0) m;
extern typeof((int *)mr) m;
extern typeof((int * restrict)mr) m;


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-19 19:34           ` Uecker, Martin
@ 2020-11-19 20:53             ` Joseph Myers
  0 siblings, 0 replies; 10+ messages in thread
From: Joseph Myers @ 2020-11-19 20:53 UTC (permalink / raw)
  To: Uecker, Martin; +Cc: gcc-patches

On Thu, 19 Nov 2020, Uecker, Martin wrote:

> Apparently I did not have enough coffee when
> generalizing this to the other qualifiers. 
> 
> Ok, with the following test?

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-19  6:34       ` [C PATCH] " Uecker, Martin
  2020-11-19 18:58         ` Joseph Myers
@ 2020-11-23 13:55         ` Christophe Lyon
  2020-11-23 15:26           ` Uecker, Martin
  1 sibling, 1 reply; 10+ messages in thread
From: Christophe Lyon @ 2020-11-23 13:55 UTC (permalink / raw)
  To: Uecker, Martin; +Cc: joseph, gcc-patches

Hi,


On Thu, 19 Nov 2020 at 07:34, Uecker, Martin
<Martin.Uecker@med.uni-goettingen.de> wrote:
>
>
>
> Here is another version of the patch. The
> only difference is the additional the check
> using 'tree_ssa_useless_type_conversion'.
>
>
> Best,
> Martin
>
>
>
>
>     C: Drop qualifiers during lvalue conversion. PR97702
>
>     2020-11-XX  Martin Uecker  <muecker@gwdg.de>
>
>     gcc/
>             * gcc/gimplify.c (gimplify_modify_expr_rhs): Optimizie
>             NOP_EXPRs that contain compound literals.
>
>     gcc/c/
>             * c-typeck.c (convert_lvalue_to_rvalue): Drop qualifiers.
>
>     gcc/testsuite/
>             * gcc.dg/cond-constqual-1.c: Adapt test.
>             * gcc.dg/lvalue-11.c: New test.
>             * gcc.dg/pr60195.c: Add warning.
>
>
>

This patch causes a regression on arm:
FAIL: gcc.dg/fixed-point/struct-union.c (test for excess errors)
Excess errors:
/gcc/testsuite/gcc.dg/fixed-point/struct-union.c:60:8: warning: value
computed is not used [-Wunused-value]
/gcc/testsuite/gcc.dg/fixed-point/struct-union.c:61:9: warning: value
computed is not used [-Wunused-value]

The test in question lines 60-61 are:
  /* Test assignment to volatile structure members.  */
  sv.f = 0.06r;
  sv.lf = 0.07lr;

so it seems these assignments are now illegally removed.

Can you check?

Thanks,

Christophe

>
>
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 413109c916c..286f3d9cd6c 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -2080,6 +2080,9 @@ convert_lvalue_to_rvalue (location_t loc, struct c_expr exp,
>      exp = default_function_array_conversion (loc, exp);
>    if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
>      exp.value = require_complete_type (loc, exp.value);
> +  if (convert_p && !error_operand_p (exp.value)
> +      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
> +    exp.value = convert (build_qualified_type (TREE_TYPE (exp.value), TYPE_UNQUALIFIED),
> exp.value);
>    if (really_atomic_lvalue (exp.value))
>      {
>        vec<tree, va_gc> *params;
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 2566ec7f0af..fd0b5202b45 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -5518,6 +5518,19 @@ gimplify_modify_expr_rhs (tree *expr_p, tree *from_p, tree *to_p,
>             return GS_OK;
>           }
>
> +       case NOP_EXPR:
> +         /* Pull out compound literal expressions from a NOP_EXPR.
> +            Those are created in the C FE to drop qualifiers during
> +            lvalue conversion.  */
> +         if ((TREE_CODE (TREE_OPERAND (*from_p, 0)) == COMPOUND_LITERAL_EXPR)
> +             && tree_ssa_useless_type_conversion (*from_p))
> +           {
> +             *from_p = TREE_OPERAND (*from_p, 0);
> +             ret = GS_OK;
> +             changed = true;
> +           }
> +         break;
> +
>         case COMPOUND_LITERAL_EXPR:
>           {
>             tree complit = TREE_OPERAND (*expr_p, 1);
> diff --git a/gcc/testsuite/gcc.dg/cond-constqual-1.c b/gcc/testsuite/gcc.dg/cond-constqual-1.c
> index 3354c7214a4..b5a09cb0038 100644
> --- a/gcc/testsuite/gcc.dg/cond-constqual-1.c
> +++ b/gcc/testsuite/gcc.dg/cond-constqual-1.c
> @@ -11,5 +11,5 @@ test (void)
>    __typeof__ (1 ? foo (0) : 0) texpr;
>    __typeof__ (1 ? i : 0) texpr2;
>    texpr = 0;  /* { dg-bogus "read-only variable" "conditional expression with call to const
> function" } */
> -  texpr2 = 0; /* { dg-error "read-only variable" "conditional expression with const variable" } */
> +  texpr2 = 0; /* { dg-bogus "read-only variable" "conditional expression with const variable" } */
>  }
> diff --git a/gcc/testsuite/gcc.dg/lvalue-11.c b/gcc/testsuite/gcc.dg/lvalue-11.c
> new file mode 100644
> index 00000000000..45a97d86890
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/lvalue-11.c
> @@ -0,0 +1,46 @@
> +/* test that lvalue conversions drops qualifiers, Bug 97702 */
> +/* { dg-do compile } */
> +/* { dg-options "" } */
> +
> +
> +void f(void)
> +{
> + const int j;
> + typeof((0,j)) i10; i10 = j;;
> + typeof(+j) i11; i11 = j;;
> + typeof(-j) i12; i12 = j;;
> + typeof(1?j:0) i13; i13 = j;;
> + typeof((int)j) i14; i14 = j;;
> + typeof((const int)j) i15; i15 = j;;
> +}
> +
> +void g(void)
> +{
> + volatile int j;
> + typeof((0,j)) i21; i21 = j;;
> + typeof(+j) i22; i22 = j;;
> + typeof(-j) i23; i23 = j;;
> + typeof(1?j:0) i24; i24 = j;;
> + typeof((int)j) i25; i25 = j;;
> + typeof((volatile int)j) i26; i26 = j;;
> +}
> +
> +void h(void)
> +{
> + _Atomic int j;
> + typeof((0,j)) i32; i32 = j;;
> + typeof(+j) i33; i33 = j;;
> + typeof(-j) i34; i34 = j;;
> + typeof(1?j:0) i35; i35 = j;;
> + typeof((int)j) i36; i36 = j;;
> + typeof((_Atomic int)j) i37; i37 = j;;
> +}
> +
> +void e(void)
> +{
> + int* restrict j;
> + typeof((0,j)) i43; i43 = j;;
> + typeof(1?j:0) i44; i44 = j;;
> + typeof((int*)j) i45; i45 = j;;
> + typeof((int* restrict)j) i46; i46 = j;;
> +}
> diff --git a/gcc/testsuite/gcc.dg/pr60195.c b/gcc/testsuite/gcc.dg/pr60195.c
> index 0a50a30be25..8eccf7f63ad 100644
> --- a/gcc/testsuite/gcc.dg/pr60195.c
> +++ b/gcc/testsuite/gcc.dg/pr60195.c
> @@ -15,7 +15,7 @@ atomic_int
>  fn2 (void)
>  {
>    atomic_int y = 0;
> -  y;
> +  y;           /* { dg-warning "statement with no effect" } */
>    return y;
>  }
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [C PATCH] Drop qualifiers during lvalue conversion
  2020-11-23 13:55         ` Christophe Lyon
@ 2020-11-23 15:26           ` Uecker, Martin
  0 siblings, 0 replies; 10+ messages in thread
From: Uecker, Martin @ 2020-11-23 15:26 UTC (permalink / raw)
  To: christophe.lyon; +Cc: gcc-patches, joseph



Am Montag, den 23.11.2020, 14:55 +0100 schrieb Christophe Lyon:
> Hi,
> 
> 
> On Thu, 19 Nov 2020 at 07:34, Uecker, Martin
> <Martin.Uecker@med.uni-goettingen.de> wrote:
> > 
> > 
> > Here is another version of the patch. The
> > only difference is the additional the check
> > using 'tree_ssa_useless_type_conversion'.
> > 
> > 
> > Best,
> > Martin
> > 
> > 
> > 
> > 
> >     C: Drop qualifiers during lvalue conversion. PR97702
> > 
> >     2020-11-XX  Martin Uecker  <muecker@gwdg.de>
> > 
> >     gcc/
> >             * gcc/gimplify.c (gimplify_modify_expr_rhs): Optimizie
> >             NOP_EXPRs that contain compound literals.
> > 
> >     gcc/c/
> >             * c-typeck.c (convert_lvalue_to_rvalue): Drop
> > qualifiers.
> > 
> >     gcc/testsuite/
> >             * gcc.dg/cond-constqual-1.c: Adapt test.
> >             * gcc.dg/lvalue-11.c: New test.
> >             * gcc.dg/pr60195.c: Add warning.
> > 
> > 
> > 
> 
> This patch causes a regression on arm:
> FAIL: gcc.dg/fixed-point/struct-union.c (test for excess errors)
> Excess errors:
> /gcc/testsuite/gcc.dg/fixed-point/struct-union.c:60:8: warning: value
> computed is not used [-Wunused-value]
> /gcc/testsuite/gcc.dg/fixed-point/struct-union.c:61:9: warning: value
> computed is not used [-Wunused-value]
> 
> The test in question lines 60-61 are:
>   /* Test assignment to volatile structure members.  */
>   sv.f = 0.06r;
>   sv.lf = 0.07lr;
> 
> so it seems these assignments are now illegally removed.
> 
> Can you check?

Must be some kind of strange interaction with _Fract.
I am not yet sure why this happens...

Best,
Martin


> Thanks,
> 
> Christophe
>
> > 
> > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> > index 413109c916c..286f3d9cd6c 100644
> > --- a/gcc/c/c-typeck.c
> > +++ b/gcc/c/c-typeck.c
> > @@ -2080,6 +2080,9 @@ convert_lvalue_to_rvalue (location_t loc,
> > struct c_expr exp,
> >      exp = default_function_array_conversion (loc, exp);
> >    if (!VOID_TYPE_P (TREE_TYPE (exp.value)))
> >      exp.value = require_complete_type (loc, exp.value);
> > +  if (convert_p && !error_operand_p (exp.value)
> > +      && (TREE_CODE (TREE_TYPE (exp.value)) != ARRAY_TYPE))
> > +    exp.value = convert (build_qualified_type (TREE_TYPE
> > (exp.value), TYPE_UNQUALIFIED),
> > exp.value);
> >    if (really_atomic_lvalue (exp.value))
> >      {
> >        vec<tree, va_gc> *params;
> > diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> > index 2566ec7f0af..fd0b5202b45 100644
> > --- a/gcc/gimplify.c
> > +++ b/gcc/gimplify.c
> > @@ -5518,6 +5518,19 @@ gimplify_modify_expr_rhs (tree *expr_p, tree
> > *from_p, tree *to_p,
> >             return GS_OK;
> >           }
> > 
> > +       case NOP_EXPR:
> > +         /* Pull out compound literal expressions from a NOP_EXPR.
> > +            Those are created in the C FE to drop qualifiers
> > during
> > +            lvalue conversion.  */
> > +         if ((TREE_CODE (TREE_OPERAND (*from_p, 0)) ==
> > COMPOUND_LITERAL_EXPR)
> > +             && tree_ssa_useless_type_conversion (*from_p))
> > +           {
> > +             *from_p = TREE_OPERAND (*from_p, 0);
> > +             ret = GS_OK;
> > +             changed = true;
> > +           }
> > +         break;
> > +
> >         case COMPOUND_LITERAL_EXPR:
> >           {
> >             tree complit = TREE_OPERAND (*expr_p, 1);
> > diff --git a/gcc/testsuite/gcc.dg/cond-constqual-1.c
> > b/gcc/testsuite/gcc.dg/cond-constqual-1.c
> > index 3354c7214a4..b5a09cb0038 100644
> > --- a/gcc/testsuite/gcc.dg/cond-constqual-1.c
> > +++ b/gcc/testsuite/gcc.dg/cond-constqual-1.c
> > @@ -11,5 +11,5 @@ test (void)
> >    __typeof__ (1 ? foo (0) : 0) texpr;
> >    __typeof__ (1 ? i : 0) texpr2;
> >    texpr = 0;  /* { dg-bogus "read-only variable" "conditional
> > expression with call to const
> > function" } */
> > -  texpr2 = 0; /* { dg-error "read-only variable" "conditional
> > expression with const variable" } */
> > +  texpr2 = 0; /* { dg-bogus "read-only variable" "conditional
> > expression with const variable" } */
> >  }
> > diff --git a/gcc/testsuite/gcc.dg/lvalue-11.c
> > b/gcc/testsuite/gcc.dg/lvalue-11.c
> > new file mode 100644
> > index 00000000000..45a97d86890
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/lvalue-11.c
> > @@ -0,0 +1,46 @@
> > +/* test that lvalue conversions drops qualifiers, Bug 97702 */
> > +/* { dg-do compile } */
> > +/* { dg-options "" } */
> > +
> > +
> > +void f(void)
> > +{
> > + const int j;
> > + typeof((0,j)) i10; i10 = j;;
> > + typeof(+j) i11; i11 = j;;
> > + typeof(-j) i12; i12 = j;;
> > + typeof(1?j:0) i13; i13 = j;;
> > + typeof((int)j) i14; i14 = j;;
> > + typeof((const int)j) i15; i15 = j;;
> > +}
> > +
> > +void g(void)
> > +{
> > + volatile int j;
> > + typeof((0,j)) i21; i21 = j;;
> > + typeof(+j) i22; i22 = j;;
> > + typeof(-j) i23; i23 = j;;
> > + typeof(1?j:0) i24; i24 = j;;
> > + typeof((int)j) i25; i25 = j;;
> > + typeof((volatile int)j) i26; i26 = j;;
> > +}
> > +
> > +void h(void)
> > +{
> > + _Atomic int j;
> > + typeof((0,j)) i32; i32 = j;;
> > + typeof(+j) i33; i33 = j;;
> > + typeof(-j) i34; i34 = j;;
> > + typeof(1?j:0) i35; i35 = j;;
> > + typeof((int)j) i36; i36 = j;;
> > + typeof((_Atomic int)j) i37; i37 = j;;
> > +}
> > +
> > +void e(void)
> > +{
> > + int* restrict j;
> > + typeof((0,j)) i43; i43 = j;;
> > + typeof(1?j:0) i44; i44 = j;;
> > + typeof((int*)j) i45; i45 = j;;
> > + typeof((int* restrict)j) i46; i46 = j;;
> > +}
> > diff --git a/gcc/testsuite/gcc.dg/pr60195.c
> > b/gcc/testsuite/gcc.dg/pr60195.c
> > index 0a50a30be25..8eccf7f63ad 100644
> > --- a/gcc/testsuite/gcc.dg/pr60195.c
> > +++ b/gcc/testsuite/gcc.dg/pr60195.c
> > @@ -15,7 +15,7 @@ atomic_int
> >  fn2 (void)
> >  {
> >    atomic_int y = 0;
> > -  y;
> > +  y;           /* { dg-warning "statement with no effect" } */
> >    return y;
> >  }
> > 

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-11-23 15:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-07 15:17 [C PATCH RFC] Drop qualifiers during lvalue conversion Uecker, Martin
2020-11-09 23:41 ` Joseph Myers
2020-11-15 20:26   ` Uecker, Martin
2020-11-16 22:50     ` Joseph Myers
2020-11-19  6:34       ` [C PATCH] " Uecker, Martin
2020-11-19 18:58         ` Joseph Myers
2020-11-19 19:34           ` Uecker, Martin
2020-11-19 20:53             ` Joseph Myers
2020-11-23 13:55         ` Christophe Lyon
2020-11-23 15:26           ` Uecker, Martin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).