RFA: PATCH to make fold_indirect_ref_1 fold more things

For constexpr I need to be able to fold some tree forms that 
fold_indirect_ref_1 didn't handle; this patch extends it to handle 
folding POINTER_PLUS_EXPR to an ARRAY_REF, and also folding to 
COMPONENT_REF.

Tested x86_64-pc-linux-gnu.  OK for trunk?

On Tue, Oct 26, 2010 at 6:35 PM, Jason Merrill <jason@redhat.com> wrote:
> For constexpr I need to be able to fold some tree forms that
> fold_indirect_ref_1 didn't handle; this patch extends it to handle folding
> POINTER_PLUS_EXPR to an ARRAY_REF, and also folding to COMPONENT_REF.
>
> Tested x86_64-pc-linux-gnu.  OK for trunk?

I think by making this kind of transforms you are prone to bugs like PR44468.

Richard.

On 10/27/2010 02:29 PM, Richard Guenther wrote:
> On Tue, Oct 26, 2010 at 6:35 PM, Jason Merrill<jason@redhat.com>  wrote:
>> For constexpr I need to be able to fold some tree forms that
>> fold_indirect_ref_1 didn't handle; this patch extends it to handle folding
>> POINTER_PLUS_EXPR to an ARRAY_REF, and also folding to COMPONENT_REF.
>
> I think by making this kind of transforms you are prone to bugs like PR44468.

Ah, interesting.  But it seems to me that this case is different because 
we are starting from an ADDR_EXPR around an expression of the aggregate 
type in the folded expression, not just any pointer to the aggregate 
type.  What do you think?

Jason

On Thu, Oct 28, 2010 at 2:45 PM, Jason Merrill <jason@redhat.com> wrote:
> On 10/27/2010 02:29 PM, Richard Guenther wrote:
>>
>> On Tue, Oct 26, 2010 at 6:35 PM, Jason Merrill<jason@redhat.com>  wrote:
>>>
>>> For constexpr I need to be able to fold some tree forms that
>>> fold_indirect_ref_1 didn't handle; this patch extends it to handle
>>> folding
>>> POINTER_PLUS_EXPR to an ARRAY_REF, and also folding to COMPONENT_REF.
>>
>> I think by making this kind of transforms you are prone to bugs like
>> PR44468.
>
> Ah, interesting.  But it seems to me that this case is different because we
> are starting from an ADDR_EXPR around an expression of the aggregate type in
> the folded expression, not just any pointer to the aggregate type.  What do
> you think?

Hm, I think what might save you is that you only look into immediate
fields (not fields
in sub-structs as we originally did).

Now I am concerned about sth like

struct S {
    int i;
    int j;
};
struct R {
    struct S a;
    int k;
};
struct S s;
int main()
{
  struct R *p = (struct R *)&s;
  s.i = 1;
  s.j = 2;
  (*(struct S *)&*p).i = 0;
  if (s.i != 0)
    abort ();
  return 0;
}

where if we end up folding the obfuscated access to p->a.i = 0 we will generate
wrong code (one might argue that *p is invalid in C, but I'm viewing this from
a middle-end POV, not a C one).  Now if &* is already folded it will look as
p and so your code wouldn't trigger, but I guess this is just a matter of
obfuscating (like using &(*p + offset)) - the point is that the actual access
would change from one via struct S to one via struct R (and the alias set
of struct is a subset of that of R, but not the other way around).

So - what kind of testcases are you trying to handle?  (the middle-end will
optimize this stuff keeping TBAA info correct during forwprop)

Richard.

> Jason
>

On 10/29/2010 11:25 AM, Richard Guenther wrote:
> So - what kind of testcases are you trying to handle?  (the middle-end will
> optimize this stuff keeping TBAA info correct during forwprop)

The RECORD_TYPE folding was for this testcase:

struct C { // literal type
   int m;
   int n;
   constexpr C(int m) : m(m), n(-m) {}
   constexpr bool is_neg() { return m < 0; }
};

constexpr bool check1(const C& c, int C:: *pm) { return c.*pm < 0; } // #1

constexpr bool check2(const C* pc, bool (C::*pm)() const) { return
(pc->*pm)(); } // #2

constexpr C c(-1);

static_assert(!check1(c, &C::n), "Error");
static_assert(check1(c, &C::m), "Error");

static_assert(check2(&c, &C::is_neg), "Error");

For constexpr we need to evaluate suitable constant expressions at 
parsing time so that we can use them as template arguments, array bounds 
and such like.

I guess I can just do this folding in the constexpr expander...

Jason

On Fri, Oct 29, 2010 at 9:34 PM, Jason Merrill <jason@redhat.com> wrote:
> On 10/29/2010 11:25 AM, Richard Guenther wrote:
>>
>> So - what kind of testcases are you trying to handle?  (the middle-end
>> will
>> optimize this stuff keeping TBAA info correct during forwprop)
>
> The RECORD_TYPE folding was for this testcase:
>
> struct C { // literal type
>  int m;
>  int n;
>  constexpr C(int m) : m(m), n(-m) {}
>  constexpr bool is_neg() { return m < 0; }
> };
>
> constexpr bool check1(const C& c, int C:: *pm) { return c.*pm < 0; } // #1
>
> constexpr bool check2(const C* pc, bool (C::*pm)() const) { return
> (pc->*pm)(); } // #2
>
> constexpr C c(-1);
>
> static_assert(!check1(c, &C::n), "Error");
> static_assert(check1(c, &C::m), "Error");
>
> static_assert(check2(&c, &C::is_neg), "Error");
>
> For constexpr we need to evaluate suitable constant expressions at parsing
> time so that we can use them as template arguments, array bounds and such
> like.
>
> I guess I can just do this folding in the constexpr expander...

Yeah, I think that should be safer.

Richard.

> Jason
>