Message ID | 1604762274.3892.1.camel@med.uni-goettingen.de |
---|---|
State | New |
Headers | show |
Series | [C,RFC] Drop qualifiers during lvalue conversion | expand |
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).
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; }
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.
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; }