[PATCH] Workaround premature bitfield folding (PR c++/79681)

[Richard Biener]

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  <jakub@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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)