Re: [C PATCH] Drop qualifiers during lvalue conversion

[Christophe Lyon <christophe.lyon@linaro.org>]

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;
>  }
>