Re: PATCH: removes expand_constant language hook

["Ollie Wild" <aaw at google dot com>]

> I'm concerned about this part.  My concern was that PTRMEM_CST is a more
> useful representation for the front end than the low-level
> representation, especially in the case of pointers to member functions.
>  Ideally, we'd like to optimize:
>
>   struct S { void f(); };
>   typedef void (S::*P)(void);
>   const P ps[2] = { &S::f, NULL };
>   void g(S s) {
>     P p = ps[0];
>     (s.*p)();
>   }
>
> into "S.f()".  I'm sure we don't do this now, but we could.

My patch actually moves a step in the right direction here.  With the
current code, the optimizer is unable to determine that p refers to
&S::f, so the assembly for g() explicitly loads ps[0].  With my patch,
compiling with -O2 or -O3 causes g() to be assembled with direct
references to S::f()

Unfortunately, the optimizer isn't smart enough to know that _ZN1S1fEv
& 1 == 0, so the function still has to test for virtual function
offsets.

More about this below.

> More concretely, does your patch handle:
>
>   struct S { void f(); };
>   typedef void (S::*P)(void);
>   const P p = &S::f;
>   template <P p> void g();
>   g<p>();


I'm not sure what is supposed to happen here.  The trunk with my
changes, the trunk without my changes, and my ubuntu gcc-4.0.3
compiler all give the same errors:

 m2.cc:5: error: 'p' cannot appear in a constant-expression
 m2.cc:5: error: expected constructor, destructor, or type conversion
before ';' token

> I'd like to be sure that the pointer-to-member constant is still
> recognized by the template machinery as a constant parameter in this
> case.  I'm not sure if digest_init gets called in this case.

The above code does work if we replace "g<p>()" with "g<&S::f>()" and
wrap the whole thing inside a function, so, yes, pointer to member
constants are still treated as constant expressions in template
parameters.  The g++ testsuite also verifies this.

> So, I'd be happier if we could do this at the point where we know we're
> actually generating the constant, which is why the hooks went into
> expand_constant in the first place.
>
> Backing up, I'm assuming that the problem you're trying to handle is
> that, for LTO, we need to avoid PTRMEM_CSTs, because we won't have
> cplus_expand_constant around to expand them for us at LTO time?  We
> could just make PTRMEM_CST a part of the middle-end -- but,
> unfortunately, expand_ptrmemfunc_cst depends on knowing how to cast to
> virtual bases, which is C++-specific.

That's part of it.  I also want to be able to include PTRMEM_CST's in
inter-file data flow and alias analysis.

For instance, if we have the following files:

 m3.h:
   struct S { void f(); };
   typedef void (S::*P)(void);
   extern const P ps[2];

 m3-1.cc:
   #include "m3.h"
   #include <cstdlib>

   const P ps[2] = { &S::f, NULL };
   void g(S s) {
     P p = ps[0];
     (s.*p)();
   }

 m3-2.cc:
   #include "m3.h"

   void h(S s) {
     P p = ps[0];
     (s.*p)();
   }

then not only should we be able to expand g(s) in s.f(), but we should
also be able to expand h(s) int s.f().  Ideally, both optimizations
should be handled identically, so we either need to pull PTRMEM_CST
into the middle end, or we need to expand PTRMEM_CST before the middle
end.  I think the latter is probably simpler, but I'm also pretty new
to this stuff, so you might know better.

> Therefore, I think the best solution would be to lower PTRMEM_CST only
> at the point of outputting LTO information.  (For PTRMEM_CSTs in the
> bodies of functions, that already happens at the point of generating
> GENERIC.)  In other words, when outputting the initializer for a global
> variable into the LTO information, call the lang_hook to expand
> constants so that the lowered information makes it to LTO.

This gets us into the scenario where one set of optimizations is used
for intra-file optimization, and another for inter-file optimization
(g and h in the example above).

Thoughts?

Ollie