Re: [PATCH RFA (fold)] c++: constexpr bit_cast with empty field
On 7/18/23 07:31, Richard Biener wrote: On Mon, Jul 17, 2023 at 11:20 PM Jason Merrill via Gcc-patches wrote: Tested x86_64-pc-linux-gnu, OK for trunk? -- 8< -- The change to only cache constexpr calls that are reduced_constant_expression_p tripped on bit-cast3.C, which failed that predicate due to the presence of an empty field in the result of native_interpret_aggregate, which reduced_constant_expression_p rejects to avoid confusing output_constructor. This patch proposes to skip such fields in native_interpret_aggregate, since they aren't actually involved in the value representation. gcc/ChangeLog: * fold-const.cc (native_interpret_aggregate): Skip empty fields. gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_bit_cast): Check that the result of native_interpret_aggregate doesn't need more evaluation. --- gcc/cp/constexpr.cc | 9 + gcc/fold-const.cc | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 9d85c3be5cc..6e8f1c2b61e 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1440,6 +1440,8 @@ enum value_cat { static tree cxx_eval_constant_expression (const constexpr_ctx *, tree, value_cat, bool *, bool *, tree * = NULL); +static tree cxx_eval_bare_aggregate (const constexpr_ctx *, tree, +value_cat, bool *, bool *); static tree cxx_fold_indirect_ref (const constexpr_ctx *, location_t, tree, tree, bool * = NULL); static tree find_heap_var_refs (tree *, int *, void *); @@ -4803,6 +4805,13 @@ cxx_eval_bit_cast (const constexpr_ctx *ctx, tree t, bool *non_constant_p, { clear_type_padding_in_mask (TREE_TYPE (t), mask); clear_uchar_or_std_byte_in_mask (loc, r, mask); + if (CHECKING_P) + { + tree e = cxx_eval_bare_aggregate (ctx, r, vc_prvalue, + non_constant_p, overflow_p); + gcc_checking_assert (e == r); + r = e; + } } } diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index a02ede79fed..db8f7de5680 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -8935,7 +8935,8 @@ native_interpret_aggregate (tree type, const unsigned char *ptr, int off, return NULL_TREE; for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) { - if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field)) + if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field) + || integer_zerop (DECL_SIZE (field))) The gimplifier uses is_empty_type (TREE_TYPE (field)), wouldn't that be better for consistency reasons at least? Good point, thanks. Pushed with that change. OK with that change or with integer_zerop if there is a reason to not use is_empty_type here. Thanks, Richard. continue; tree fld = field; HOST_WIDE_INT bitoff = 0, pos = 0, sz = 0; base-commit: caabf0973a4e9a26421c94d540e3e20051e93e77 -- 2.39.3
Re: [PATCH RFA (fold)] c++: constexpr bit_cast with empty field
On Mon, Jul 17, 2023 at 11:20 PM Jason Merrill via Gcc-patches wrote: > > Tested x86_64-pc-linux-gnu, OK for trunk? > > -- 8< -- > > The change to only cache constexpr calls that are > reduced_constant_expression_p tripped on bit-cast3.C, which failed that > predicate due to the presence of an empty field in the result of > native_interpret_aggregate, which reduced_constant_expression_p rejects to > avoid confusing output_constructor. > > This patch proposes to skip such fields in native_interpret_aggregate, since > they aren't actually involved in the value representation. > > gcc/ChangeLog: > > * fold-const.cc (native_interpret_aggregate): Skip empty fields. > > gcc/cp/ChangeLog: > > * constexpr.cc (cxx_eval_bit_cast): Check that the result of > native_interpret_aggregate doesn't need more evaluation. > --- > gcc/cp/constexpr.cc | 9 + > gcc/fold-const.cc | 3 ++- > 2 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc > index 9d85c3be5cc..6e8f1c2b61e 100644 > --- a/gcc/cp/constexpr.cc > +++ b/gcc/cp/constexpr.cc > @@ -1440,6 +1440,8 @@ enum value_cat { > > static tree cxx_eval_constant_expression (const constexpr_ctx *, tree, > value_cat, bool *, bool *, tree * = > NULL); > +static tree cxx_eval_bare_aggregate (const constexpr_ctx *, tree, > +value_cat, bool *, bool *); > static tree cxx_fold_indirect_ref (const constexpr_ctx *, location_t, tree, > tree, >bool * = NULL); > static tree find_heap_var_refs (tree *, int *, void *); > @@ -4803,6 +4805,13 @@ cxx_eval_bit_cast (const constexpr_ctx *ctx, tree t, > bool *non_constant_p, > { > clear_type_padding_in_mask (TREE_TYPE (t), mask); > clear_uchar_or_std_byte_in_mask (loc, r, mask); > + if (CHECKING_P) > + { > + tree e = cxx_eval_bare_aggregate (ctx, r, vc_prvalue, > + non_constant_p, overflow_p); > + gcc_checking_assert (e == r); > + r = e; > + } > } > } > > diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc > index a02ede79fed..db8f7de5680 100644 > --- a/gcc/fold-const.cc > +++ b/gcc/fold-const.cc > @@ -8935,7 +8935,8 @@ native_interpret_aggregate (tree type, const unsigned > char *ptr, int off, > return NULL_TREE; >for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) > { > - if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field)) > + if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field) > + || integer_zerop (DECL_SIZE (field))) The gimplifier uses is_empty_type (TREE_TYPE (field)), wouldn't that be better for consistency reasons at least? OK with that change or with integer_zerop if there is a reason to not use is_empty_type here. Thanks, Richard. > continue; >tree fld = field; >HOST_WIDE_INT bitoff = 0, pos = 0, sz = 0; > > base-commit: caabf0973a4e9a26421c94d540e3e20051e93e77 > -- > 2.39.3 >
[PATCH RFA (fold)] c++: constexpr bit_cast with empty field
Tested x86_64-pc-linux-gnu, OK for trunk? -- 8< -- The change to only cache constexpr calls that are reduced_constant_expression_p tripped on bit-cast3.C, which failed that predicate due to the presence of an empty field in the result of native_interpret_aggregate, which reduced_constant_expression_p rejects to avoid confusing output_constructor. This patch proposes to skip such fields in native_interpret_aggregate, since they aren't actually involved in the value representation. gcc/ChangeLog: * fold-const.cc (native_interpret_aggregate): Skip empty fields. gcc/cp/ChangeLog: * constexpr.cc (cxx_eval_bit_cast): Check that the result of native_interpret_aggregate doesn't need more evaluation. --- gcc/cp/constexpr.cc | 9 + gcc/fold-const.cc | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc index 9d85c3be5cc..6e8f1c2b61e 100644 --- a/gcc/cp/constexpr.cc +++ b/gcc/cp/constexpr.cc @@ -1440,6 +1440,8 @@ enum value_cat { static tree cxx_eval_constant_expression (const constexpr_ctx *, tree, value_cat, bool *, bool *, tree * = NULL); +static tree cxx_eval_bare_aggregate (const constexpr_ctx *, tree, +value_cat, bool *, bool *); static tree cxx_fold_indirect_ref (const constexpr_ctx *, location_t, tree, tree, bool * = NULL); static tree find_heap_var_refs (tree *, int *, void *); @@ -4803,6 +4805,13 @@ cxx_eval_bit_cast (const constexpr_ctx *ctx, tree t, bool *non_constant_p, { clear_type_padding_in_mask (TREE_TYPE (t), mask); clear_uchar_or_std_byte_in_mask (loc, r, mask); + if (CHECKING_P) + { + tree e = cxx_eval_bare_aggregate (ctx, r, vc_prvalue, + non_constant_p, overflow_p); + gcc_checking_assert (e == r); + r = e; + } } } diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index a02ede79fed..db8f7de5680 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -8935,7 +8935,8 @@ native_interpret_aggregate (tree type, const unsigned char *ptr, int off, return NULL_TREE; for (tree field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) { - if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field)) + if (TREE_CODE (field) != FIELD_DECL || DECL_PADDING_P (field) + || integer_zerop (DECL_SIZE (field))) continue; tree fld = field; HOST_WIDE_INT bitoff = 0, pos = 0, sz = 0; base-commit: caabf0973a4e9a26421c94d540e3e20051e93e77 -- 2.39.3