Re: [PATCH] c++: constexpr base-to-derived conversion with offset 0 [PR103879]

[Jason Merrill <jason@redhat.com>]

On 1/4/22 11:54, Patrick Palka wrote:
> r12-136 made us canonicalize an object/offset pair with negative offset
> into one with a nonnegative offset, by iteratively absorbing the
> innermost component into the offset and stopping as soon as the offset
> becomes nonnegative.
> 
> This patch strengthens this transformation to make it keep absorbing
> even if the offset is already 0 as long as the innermost component is at
> position 0 (and thus absorbing doesn't change the offset).  This lets us
> accept the two constexpr testcases below, which we'd previously reject
> essentially because cxx_fold_indirect_ref wasn't able to resolve
> *(B*)&b.D123 (where D123 is the base subobject A at position 0) to just b.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/103879
> 
> gcc/cp/ChangeLog:
> 
> 	* constexpr.c (cxx_fold_indirect_ref): Split out object/offset
> 	canonicalization step into a local lambda.  Strengthen it to
> 	absorb more components at position 0.  Use it before both calls
> 	to cxx_fold_indirect_ref_1.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1y/constexpr-base2.C: New test.
> 	* g++.dg/cpp1y/constexpr-base2a.C: New test.
> ---
>   gcc/cp/constexpr.c                            | 38 +++++++++++++------
>   gcc/testsuite/g++.dg/cpp1y/constexpr-base2.C  | 21 ++++++++++
>   gcc/testsuite/g++.dg/cpp1y/constexpr-base2a.C | 25 ++++++++++++
>   3 files changed, 72 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-base2.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-base2a.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 72be45c9e87..1ec33a00ee5 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -5144,6 +5144,25 @@ cxx_fold_indirect_ref (const constexpr_ctx *ctx, location_t loc, tree type,
>     if (!INDIRECT_TYPE_P (subtype))
>       return NULL_TREE;
>   
> +  /* Canonicalizes the given OBJ/OFF pair by iteratively absorbing
> +     the innermost component into the offset until the offset is
> +     nonnegative,

Maybe "until it would make the offset positive" now that you continue 
with repeated zeros.  OK with that change.

> so that cxx_fold_indirect_ref_1 can identify
> +     more folding opportunities.  */
> +  auto canonicalize_obj_off = [] (tree& obj, tree& off) {
> +    while (TREE_CODE (obj) == COMPONENT_REF
> +	   && (tree_int_cst_sign_bit (off) || integer_zerop (off)))
> +      {
> +	tree field = TREE_OPERAND (obj, 1);
> +	tree pos = byte_position (field);
> +	if (integer_zerop (off) && integer_nonzerop (pos))
> +	  /* If the offset is already 0, keep going as long as the
> +	     component is at position 0.  */
> +	  break;
> +	off = int_const_binop (PLUS_EXPR, off, pos);
> +	obj = TREE_OPERAND (obj, 0);
> +      }
> +  };
>
>     if (TREE_CODE (sub) == ADDR_EXPR)
>       {
>         tree op = TREE_OPERAND (sub, 0);
> @@ -5162,7 +5181,12 @@ cxx_fold_indirect_ref (const constexpr_ctx *ctx, location_t loc, tree type,
>   	    return op;
>   	}
>         else
> -	return cxx_fold_indirect_ref_1 (ctx, loc, type, op, 0, empty_base);
> +	{
> +	  tree off = integer_zero_node;
> +	  canonicalize_obj_off (op, off);
> +	  gcc_assert (integer_zerop (off));
> +	  return cxx_fold_indirect_ref_1 (ctx, loc, type, op, 0, empty_base);
> +	}
>       }
>     else if (TREE_CODE (sub) == POINTER_PLUS_EXPR
>   	   && tree_fits_uhwi_p (TREE_OPERAND (sub, 1)))
> @@ -5174,17 +5198,7 @@ cxx_fold_indirect_ref (const constexpr_ctx *ctx, location_t loc, tree type,
>         if (TREE_CODE (op00) == ADDR_EXPR)
>   	{
>   	  tree obj = TREE_OPERAND (op00, 0);
> -	  while (TREE_CODE (obj) == COMPONENT_REF
> -		 && tree_int_cst_sign_bit (off))
> -	    {
> -	      /* Canonicalize this object/offset pair by iteratively absorbing
> -		 the innermost component into the offset until the offset is
> -		 nonnegative, so that cxx_fold_indirect_ref_1 can identify
> -		 more folding opportunities.  */
> -	      tree field = TREE_OPERAND (obj, 1);
> -	      off = int_const_binop (PLUS_EXPR, off, byte_position (field));
> -	      obj = TREE_OPERAND (obj, 0);
> -	    }
> +	  canonicalize_obj_off (obj, off);
>   	  return cxx_fold_indirect_ref_1 (ctx, loc, type, obj,
>   					  tree_to_uhwi (off), empty_base);
>   	}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-base2.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-base2.C
> new file mode 100644
> index 00000000000..7cbf5bf32b7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-base2.C
> @@ -0,0 +1,21 @@
> +// PR c++/103879
> +// { dg-do compile { target c++14 } }
> +
> +struct A {
> +  int n = 42;
> +};
> +
> +struct B : A { };
> +
> +struct C {
> +  B b;
> +};
> +
> +constexpr int f() {
> +  C c;
> +  A& a = static_cast<A&>(c.b);
> +  B& b = static_cast<B&>(a);
> +  return b.n;
> +}
> +
> +static_assert(f() == 42, "");
> diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-base2a.C b/gcc/testsuite/g++.dg/cpp1y/constexpr-base2a.C
> new file mode 100644
> index 00000000000..872e9bb6d6a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-base2a.C
> @@ -0,0 +1,25 @@
> +// PR c++/103879
> +// { dg-do compile { target c++14 } }
> +
> +struct A {
> +  int n = 42;
> +};
> +
> +struct Y { int m = 0; };
> +
> +struct X : Y, A { };
> +
> +struct B : X { };
> +
> +struct C {
> +  B b;
> +};
> +
> +constexpr int f() {
> +  C c;
> +  A& a = static_cast<A&>(c.b);
> +  B& b = static_cast<B&>(a);
> +  return b.n;
> +}
> +
> +static_assert(f() == 42, "");