[C++ PATCH] Fix offsetof constexpr handling (PR c++/85662, take 4)
Jason Merrill
jason@redhat.com
Thu May 10 15:24:00 GMT 2018
OK, thanks.
On Wed, May 9, 2018 at 4:49 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, May 09, 2018 at 11:01:18AM -0400, Jason Merrill wrote:
>> On Wed, May 9, 2018 at 10:47 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> > On Wed, May 09, 2018 at 10:40:26AM -0400, Jason Merrill wrote:
>> >> On Wed, May 9, 2018 at 4:55 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>> >> > On Tue, May 08, 2018 at 11:28:18PM -0400, Jason Merrill wrote:
>> >> >> Maybe add a type parameter that defaults to size_type_node...
>> >> >>
>> >> >> > + ret = fold_convert_loc (loc, TREE_TYPE (expr),
>> >> >> > + fold_offsetof_1 (TREE_TYPE (expr), op0));
>> >> >>
>> >> >> ...and then this can be
>> >> >>
>> >> >> fold_offsetof (op0, TREE_TYPE (exp0))
>> >> >
>> >> > Like this then?
>> >> >
>> >> > + ret = fold_convert_loc (loc, TREE_TYPE (expr),
>> >> > + fold_offsetof (op0, TREE_TYPE (expr)));
>> >>
>> >> I was thinking that we then wouldn't need the fold_convert at the call
>> >> sites anymore, either.
>> >
>> > The patch only converts to non-pointer types, I'm not sure if it is
>> > desirable to do the same with pointer types (and most of the other callers
>> > don't use convert, but fold_convert which is significantly different, the
>> > former is emitting diagnostics, the latter is just an conversion + optimization).
>>
>> Is there a reason we can't use fold_convert for the non-pointer case,
>> too? I don't think we're interested in diagnostics from this
>> particular call.
>
> This patch instead uses convert everywhere. Bootstrapped/regtested on
> x86_64-linux and i686-linux, ok for trunk?
>
> 2018-05-09 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/85662
> * c-common.h (fold_offsetof_1): Removed.
> (fold_offsetof): Add TYPE argument defaulted to size_type_node and
> CTX argument defaulted to ERROR_MARK.
> * c-common.c (fold_offsetof_1): Renamed to ...
> (fold_offsetof): ... this. Remove wrapper function. Add TYPE
> argument, convert the pointer constant to TYPE and use size_binop
> with PLUS_EXPR instead of fold_build_pointer_plus if type is not
> a pointer type. Adjust recursive calls.
>
> * c-fold.c (c_fully_fold_internal): Use fold_offsetof rather than
> fold_offsetof_1, pass TREE_TYPE (expr) as TYPE to it and drop the
> fold_convert_loc.
> * c-typeck.c (build_unary_op): Use fold_offsetof rather than
> fold_offsetof_1, pass argtype as TYPE to it and drop the
> fold_convert_loc.
>
> * cp-gimplify.c (cp_fold): Use fold_offsetof rather than
> fold_offsetof_1, pass TREE_TYPE (x) as TYPE to it and drop the
> fold_convert.
>
> * g++.dg/ext/offsetof2.C: New test.
>
> --- gcc/c-family/c-common.h.jj 2018-05-09 20:12:25.845258371 +0200
> +++ gcc/c-family/c-common.h 2018-05-09 20:20:02.265649121 +0200
> @@ -1033,8 +1033,8 @@ extern bool c_dump_tree (void *, tree);
>
> extern void verify_sequence_points (tree);
>
> -extern tree fold_offsetof_1 (tree, tree_code ctx = ERROR_MARK);
> -extern tree fold_offsetof (tree);
> +extern tree fold_offsetof (tree, tree = size_type_node,
> + tree_code ctx = ERROR_MARK);
>
> extern int complete_array_type (tree *, tree, bool);
>
> --- gcc/c-family/c-common.c.jj 2018-05-09 20:12:25.763258297 +0200
> +++ gcc/c-family/c-common.c 2018-05-09 20:21:23.770718896 +0200
> @@ -6168,10 +6168,11 @@ c_common_to_target_charset (HOST_WIDE_IN
>
> /* Fold an offsetof-like expression. EXPR is a nested sequence of component
> references with an INDIRECT_REF of a constant at the bottom; much like the
> - traditional rendering of offsetof as a macro. Return the folded result. */
> + traditional rendering of offsetof as a macro. TYPE is the desired type of
> + the whole expression. Return the folded result. */
>
> tree
> -fold_offsetof_1 (tree expr, enum tree_code ctx)
> +fold_offsetof (tree expr, tree type, enum tree_code ctx)
> {
> tree base, off, t;
> tree_code code = TREE_CODE (expr);
> @@ -6196,10 +6197,10 @@ fold_offsetof_1 (tree expr, enum tree_co
> error ("cannot apply %<offsetof%> to a non constant address");
> return error_mark_node;
> }
> - return TREE_OPERAND (expr, 0);
> + return convert (type, TREE_OPERAND (expr, 0));
>
> case COMPONENT_REF:
> - base = fold_offsetof_1 (TREE_OPERAND (expr, 0), code);
> + base = fold_offsetof (TREE_OPERAND (expr, 0), type, code);
> if (base == error_mark_node)
> return base;
>
> @@ -6216,7 +6217,7 @@ fold_offsetof_1 (tree expr, enum tree_co
> break;
>
> case ARRAY_REF:
> - base = fold_offsetof_1 (TREE_OPERAND (expr, 0), code);
> + base = fold_offsetof (TREE_OPERAND (expr, 0), type, code);
> if (base == error_mark_node)
> return base;
>
> @@ -6273,23 +6274,16 @@ fold_offsetof_1 (tree expr, enum tree_co
> /* Handle static members of volatile structs. */
> t = TREE_OPERAND (expr, 1);
> gcc_checking_assert (VAR_P (get_base_address (t)));
> - return fold_offsetof_1 (t);
> + return fold_offsetof (t, type);
>
> default:
> gcc_unreachable ();
> }
>
> + if (!POINTER_TYPE_P (type))
> + return size_binop (PLUS_EXPR, base, convert (type, off));
> return fold_build_pointer_plus (base, off);
> }
> -
> -/* Likewise, but convert it to the return type of offsetof. */
> -
> -tree
> -fold_offsetof (tree expr)
> -{
> - return convert (size_type_node, fold_offsetof_1 (expr));
> -}
> -
>
> /* *PTYPE is an incomplete array. Complete it with a domain based on
> INITIAL_VALUE. If INITIAL_VALUE is not present, use 1 if DO_DEFAULT
> --- gcc/c/c-fold.c.jj 2018-05-09 20:12:25.496258072 +0200
> +++ gcc/c/c-fold.c 2018-05-09 20:21:53.673744504 +0200
> @@ -473,7 +473,7 @@ c_fully_fold_internal (tree expr, bool i
> && (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));
> + ret = fold_offsetof (op0, TREE_TYPE (expr));
> else if (op0 != orig_op0 || in_init)
> ret = in_init
> ? fold_build1_initializer_loc (loc, code, TREE_TYPE (expr), op0)
> --- gcc/c/c-typeck.c.jj 2018-05-09 20:12:25.529258100 +0200
> +++ gcc/c/c-typeck.c 2018-05-09 20:22:10.592758986 +0200
> @@ -4676,7 +4676,7 @@ build_unary_op (location_t location, enu
> if (val && INDIRECT_REF_P (val)
> && TREE_CONSTANT (TREE_OPERAND (val, 0)))
> {
> - ret = fold_convert_loc (location, argtype, fold_offsetof_1 (arg));
> + ret = fold_offsetof (arg, argtype);
> goto return_build_unary_op;
> }
>
> --- gcc/cp/cp-gimplify.c.jj 2018-05-09 20:12:25.881258402 +0200
> +++ gcc/cp/cp-gimplify.c 2018-05-09 20:22:35.639780430 +0200
> @@ -2232,7 +2232,7 @@ cp_fold (tree x)
> val = TREE_OPERAND (val, 0);
> STRIP_NOPS (val);
> if (TREE_CODE (val) == INTEGER_CST)
> - return fold_convert (TREE_TYPE (x), fold_offsetof_1 (op0));
> + return fold_offsetof (op0, TREE_TYPE (x));
> }
> }
> goto finish_unary;
> --- gcc/testsuite/g++.dg/ext/offsetof2.C.jj 2018-05-09 20:20:02.272649127 +0200
> +++ gcc/testsuite/g++.dg/ext/offsetof2.C 2018-05-09 20:20:02.272649127 +0200
> @@ -0,0 +1,6 @@
> +// PR c++/85662
> +// { dg-do compile { target c++11 } }
> +
> +struct S { unsigned long x[31]; };
> +struct T { bool b; S f; };
> +static_assert (__builtin_offsetof (T, f.x[31 - 1]) == __builtin_offsetof (T, f.x[30]), "");
>
>
> Jakub
More information about the Gcc-patches
mailing list