On Tue, 28 Feb 2017, Jakub Jelinek wrote:
> Hi!
>
> make_bit_field_ref and its callers are invoked not just during GIMPLE (where
> at least fold_truth_andor* isn't really effective; but from what I've tested
> it doesn't do a good job in many cases at GENERIC either, because we
> fold x.btfld == y.btfld too early into ((BIT_FIELD_REF ^ BIT_FIELD_REF) &
> cst) == 0
> or similar and fold_truth_andor* can't handle that), but also during GENERIC
> and strip away the whole component access path if there are only constant
> offsets (various nested COMPONENT_REFs as well as ARRAY_REFs). That is not
> really good for constexpr evaluation (we should evaluate constexpr on saved
> fn trees before folding, but that is not GCC7 material), which then can't
> locate where to look for the value.
>
> This patch just attempts to reuse as much as possible from orig_inner and
> only use adjusted BIT_FIELD_REF on that when in GENERIC (first I've tried to
> use instead COMPONENT_REF with DECL_BIT_FIELD_REPRESENTATIVE, but that isn't
> handled by constexpr.c either).
> Bootstrapped/regtested on x86_64/i686-linux, ok for trunk?
Hmm, I'd rather not do anything is_gimple_form specific. Can't we
restrict this transform to work on the outermost handled-component only?
Ideally we'd want to find a common base of course.
The folding is also guarded with optimize != 0 so maybe simply disable
it from the FEs? (but yes, you say it does a bad job on GIMPLE
which is quite understandable as there's no TRUTH_*, but we can
eventually call it from gimplification instead?)
Bah, just not really liking these kind of "hacks"...
Richaard.
> 2017-02-28 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/79681
> * fold-const.c (make_bit_field_ref): During FE parsing if orig_inner
> is COMPONENT_REF, attempt to use its first operand as BIT_FIELD_REF
> base.
>
> * g++.dg/cpp1y/constexpr-79681-1.C: New test.
> * g++.dg/cpp1y/constexpr-79681-2.C: New test.
>
> --- gcc/fold-const.c.jj 2017-02-17 18:29:24.000000000 +0100
> +++ gcc/fold-const.c 2017-02-27 15:05:43.743266260 +0100
> @@ -3862,6 +3862,31 @@ make_bit_field_ref (location_t loc, tree
> {
> tree result, bftype;
>
> + /* Attempt not to lose access path if possible during FE folding. */
> + if (!in_gimple_form && TREE_CODE (orig_inner) == COMPONENT_REF)
> + {
> + tree ninner = TREE_OPERAND (orig_inner, 0);
> + machine_mode nmode;
> + HOST_WIDE_INT nbitsize, nbitpos;
> + tree noffset;
> + int nunsignedp, nreversep, nvolatilep = 0;
> + tree base = get_inner_reference (ninner, &nbitsize, &nbitpos,
> + &noffset, &nmode, &nunsignedp,
> + &nreversep, &nvolatilep);
> + if (base == inner
> + && noffset == NULL_TREE
> + && nbitsize >= bitsize
> + && nbitpos <= bitpos
> + && bitpos + bitsize <= nbitpos + nbitsize
> + && !reversep
> + && !nreversep
> + && !nvolatilep)
> + {
> + inner = ninner;
> + bitpos -= nbitpos;
> + }
> + }
> +
> alias_set_type iset = get_alias_set (orig_inner);
> if (iset == 0 && get_alias_set (inner) != iset)
> inner = fold_build2 (MEM_REF, TREE_TYPE (inner),
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-79681-1.C.jj 2017-02-27
> 15:11:39.177589060 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-79681-1.C 2017-02-27
> 15:11:28.000000000 +0100
> @@ -0,0 +1,17 @@
> +// PR c++/79681
> +// { dg-do compile { target c++14 } }
> +// { dg-options "-O2" }
> +
> +struct A
> +{
> + int i : 4;
> +};
> +
> +constexpr bool
> +foo ()
> +{
> + A x[] = { 1 };
> + return x[0].i;
> +}
> +
> +static_assert (foo(), "");
> --- gcc/testsuite/g++.dg/cpp1y/constexpr-79681-2.C.jj 2017-02-27
> 15:11:42.252548562 +0100
> +++ gcc/testsuite/g++.dg/cpp1y/constexpr-79681-2.C 2017-02-27
> 15:10:13.000000000 +0100
> @@ -0,0 +1,39 @@
> +// PR c++/79681
> +// { dg-do compile { target c++14 } }
> +// { dg-options "-O2" }
> +
> +struct A
> +{
> + char i : 4;
> + char k : 1;
> + char l : 3;
> +};
> +struct B
> +{
> + char j : 4;
> +};
> +struct C
> +{
> + long long u;
> + A a[1];
> + B b[1];
> +};
> +
> +constexpr bool
> +foo ()
> +{
> + C c = { 0, { { 5, 0, 2 } }, { { 6 } } };
> + C d = { 0, { { 6, 0, 1 } }, { { 5 } } };
> + return c.a[0].i == d.a[0].i && c.b[0].j == d.b[0].j;
> +}
> +
> +constexpr bool
> +bar ()
> +{
> + C c = { 0, { { 5, 0, 2 } }, { { 6 } } };
> + C d = { 0, { { 6, 0, 1 } }, { { 5 } } };
> + return c.a[0].i == d.a[0].i && c.a[0].l == d.a[0].l;
> +}
> +
> +static_assert (foo () == false, "");
> +static_assert (bar () == false, "");
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
