On Mon, 26 Apr 2021, Jason Merrill wrote:
> On 4/26/21 12:17 PM, Patrick Palka wrote:
> > During constexpr evaluation, a base-to-derived conversion may yield an
> > expression like (Derived*)(&D.2217.D.2106 p+ -4) where D.2217 is the
> > derived object and D.2106 is the base. But cxx_fold_indirect_ref
> > doesn't know how to resolve an INDIRECT_REF thereof to just D.2217,
> > because it doesn't handle POINTER_PLUS_EXPRs of a COMPONENT_REF and
> > negative offset well: when the offset N is positive, it knows that
> > '&x p+ N' is equivalent to '&x.f p+ (N - bytepos(f))', but it doesn't
> > know about the reverse transformation, that '&x.f p+ N' is equivalent
> > to '&x p+ (N + bytepos(f))' when N is negative, which is important for
> > resolving such base-to-derived conversions and for accessing subobjects
> > backwards. This patch teaches cxx_fold_indirect_ref this reverse
> > transformation.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
>
> OK. Do you think it's worth doing this in fold-const.c:fold_indirect_ref_1 as
> well?
Hmm, I'm not sure. The function comment for cxx_fold_indirect_ref says
"Also we want to allow folding to COMPONENT_REF, which would cause
trouble with TBAA in fold_indirect_ref_1", presumably referring to the
folding of '*(&x p+ N)' to 'x.f', so I suppose the reverse folding
'*(&x.f p+ -N)' to 'x' might be problematic for similar reasons. But
I'm not sure why the first folding is problematic for TBAA in the first
place.
>
> > gcc/cp/ChangeLog:
> >
> > PR c++/100209
> > * constexpr.c (cxx_fold_indirect_ref): Try to canonicalize the
> > object/offset pair for a POINTER_PLUS_EXPR of a COMPONENT_REF
> > with a negative offset into one whose offset is nonnegative
> > before calling cxx_fold_indirect_ref_1.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/100209
> > * g++.dg/cpp0x/constexpr-ptrsub3.C: New test.
> > * g++.dg/cpp1y/constexpr-base1.C: New test.
> > ---
> > gcc/cp/constexpr.c | 20 +++++++++++--
> > .../g++.dg/cpp0x/constexpr-ptrsub3.C | 23 +++++++++++++++
> > gcc/testsuite/g++.dg/cpp1y/constexpr-base1.C | 28 +++++++++++++++++++
> > 3 files changed, 68 insertions(+), 3 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ptrsub3.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp1y/constexpr-base1.C
> >
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 0fb0ab44b39..fa7eaed945a 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -4894,12 +4894,26 @@ cxx_fold_indirect_ref (const constexpr_ctx *ctx,
> > location_t loc, tree type,
> > && tree_fits_uhwi_p (TREE_OPERAND (sub, 1)))
> > {
> > tree op00 = TREE_OPERAND (sub, 0);
> > - tree op01 = TREE_OPERAND (sub, 1);
> > + tree off = TREE_OPERAND (sub, 1);
> > STRIP_NOPS (op00);
> > if (TREE_CODE (op00) == ADDR_EXPR)
> > - return cxx_fold_indirect_ref_1 (ctx, loc, type, TREE_OPERAND (op00,
> > 0),
> > - tree_to_uhwi (op01), empty_base);
> > + {
> > + 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);
> > + }
> > + return cxx_fold_indirect_ref_1 (ctx, loc, type, obj,
> > + tree_to_uhwi (off), empty_base);
> > + }
> > }
> > /* *(foo *)fooarrptr => (*fooarrptr)[0] */
> > else if (TREE_CODE (TREE_TYPE (subtype)) == ARRAY_TYPE
> > diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrsub3.C
> > b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrsub3.C
> > new file mode 100644
> > index 00000000000..6769f7b3617
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrsub3.C
> > @@ -0,0 +1,23 @@
> > +// PR c++/100209
> > +// { dg-do compile { target c++11 } }
> > +
> > +struct A {
> > + int x = 1;
> > +};
> > +
> > +struct B : A {
> > + int y = 2;
> > + int z = 3;
> > + int w = 4;
> > +};
> > +
> > +constexpr bool f() {
> > + B b;
> > + if (&b.w - &b.x != 3)
> > + /* Effectively disable this test if the layout of B is not what we
> > + expect. */
> > + return true;
> > + const int* w = &b.w;
> > + return *w-- == 4 && *w-- == 3 && *w-- == 2 && *w-- == 1;
> > +}
> > +static_assert(f(), "");
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/constexpr-base1.C
> > b/gcc/testsuite/g++.dg/cpp1y/constexpr-base1.C
> > new file mode 100644
> > index 00000000000..3c93aa851f3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/constexpr-base1.C
> > @@ -0,0 +1,28 @@
> > +// PR c++/100209
> > +// { dg-do compile { target c++14 } }
> > +
> > +template<typename Derived>
> > +struct __a_t
> > +{
> > + unsigned char A = 0;
> > + constexpr Derived & SetA(const unsigned char & value) {
> > + A = value;
> > + return *static_cast<Derived *>(this);
> > + }
> > +};
> > +
> > +template<typename Derived>
> > +struct __b_t
> > +{
> > + unsigned char B = 0;
> > + constexpr Derived & SetB(const unsigned char & value) {
> > + B = value;
> > + return *static_cast<Derived *>(this);
> > + }
> > +};
> > +
> > +struct __ab_t : __a_t<__ab_t>, __b_t<__ab_t> { };
> > +
> > +constexpr auto AB = __ab_t().SetA(100).SetB(10);
> > +static_assert(AB.A == 100, "");
> > +static_assert(AB.B == 10, "");
> >
>
>
