| Message ID | 20110627163937.6C7221DA195@topo.tor.corp.google.com |
|---|---|
| State | New |
| Headers | show |
Just wondering why you're naming x finishing by an underscore "x_", this is a valid name, but just thinking it's tricky syntax, does this test anything more? or is it just a preference for private members? LGTM On Mon, Jun 27, 2011 at 9:39 AM, Diego Novillo <dnovillo@google.com> wrote: > We are very close to having the first simple C++ program working from > a PPH image. Adding a runnable test case. > > This produces assembly miscomparisons and does not link. I'll be > working on it this week. > > > Diego. > > * g++.dg/pph/x1ten-hellos.cc: New. > * g++.dg/pph/x1ten-hellos.h: New. > * g++.dg/pph/pph.map: Add x1ten-hellos.h > > diff --git a/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc b/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc > new file mode 100644 > index 0000000..b99d08a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc > @@ -0,0 +1,12 @@ > +// { dg-do run } > +// { dg-xfail-if "LINK ERROR" { "*-*-*" } { "-fpph-map=pph.map" } } > +// pph asm xdiff > +#include "x1ten-hellos.h" > + > +int main(void) > +{ > + A a; > + for (int i = 0; i < 10; i++) > + a.hello(); > + return 0; > +} > diff --git a/gcc/testsuite/g++.dg/pph/x1ten-hellos.h b/gcc/testsuite/g++.dg/pph/x1ten-hellos.h > new file mode 100644 > index 0000000..2a53b66 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/pph/x1ten-hellos.h > @@ -0,0 +1,21 @@ > +#ifndef A_H_ > +#define A_H_ > +#include <stdio.h> > + > +class A > +{ > +public: > + A() { > + x_ = 0; > + printf ("constructing\n"); > + } > + > + void hello(void) { > + x_++; > + printf ("Hello World (%d)\n", x_); > + } > + > +private: > + int x_; > +}; > +#endif > diff --git a/gcc/testsuite/g++.dg/pph/pph.map b/gcc/testsuite/g++.dg/pph/pph.map > index f0c7abd..2735af8 100644 > --- a/gcc/testsuite/g++.dg/pph/pph.map > +++ b/gcc/testsuite/g++.dg/pph/pph.map > @@ -38,6 +47,7 @@ x1struct0.h x1struct0.pph > x1struct1.h x1struct1.pph > x1struct2.h x1struct2.pph > x1template.h x1template.pph > +x1ten-hellos.h x1ten-hellos.pph > x1tmplclass.h x1tmplclass.pph > x1tmplfunc.h x1tmplfunc.pph > x1typerefs.h x1typerefs.pph > > -- > This patch is available for review at http://codereview.appspot.com/4629075 >
On Mon, Jun 27, 2011 at 13:06, Gabriel Charette <gchare@google.com> wrote: > Just wondering why you're naming x finishing by an underscore "x_", > this is a valid name, but just thinking it's tricky syntax, does this > test anything more? or is it just a preference for private members? No real reason. Just a quick hack I was trying to compile with pph enabled.
On 6/27/11, Diego Novillo <dnovillo@google.com> wrote: > On Jun 27, 2011 Gabriel Charette <gchare@google.com> wrote: > > Just wondering why you're naming x finishing by an underscore > > "x_", this is a valid name, but just thinking it's tricky syntax, > > does this test anything more? or is it just a preference for > > private members? > > No real reason. Just a quick hack I was trying to compile with > pph enabled. There is a convention in Google (and elsewhere) to mark data member fields with a trailing underscore in their name. This convention makes it easy to identify field accesses, which helps avoid performance problems (unnecessary reloads) and correctness problems (missing reloads in the presence of aliasing).
On Mon, Jun 27, 2011 at 14:56, Lawrence Crowl <crowl@google.com> wrote: > On 6/27/11, Diego Novillo <dnovillo@google.com> wrote: >> On Jun 27, 2011 Gabriel Charette <gchare@google.com> wrote: >> > Just wondering why you're naming x finishing by an underscore >> > "x_", this is a valid name, but just thinking it's tricky syntax, >> > does this test anything more? or is it just a preference for >> > private members? >> >> No real reason. Just a quick hack I was trying to compile with >> pph enabled. > > There is a convention in Google (and elsewhere) to mark data member > fields with a trailing underscore in their name. This convention > makes it easy to identify field accesses, which helps avoid > performance problems (unnecessary reloads) and correctness problems > (missing reloads in the presence of aliasing). That's probably where I picked it from. I wasn't really thinking about the code itself. Just wanted to get some simple C++ executable test. Diego.
diff --git a/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc b/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc new file mode 100644 index 0000000..b99d08a --- /dev/null +++ b/gcc/testsuite/g++.dg/pph/x1ten-hellos.cc @@ -0,0 +1,12 @@ +// { dg-do run } +// { dg-xfail-if "LINK ERROR" { "*-*-*" } { "-fpph-map=pph.map" } } +// pph asm xdiff +#include "x1ten-hellos.h" + +int main(void) +{ + A a; + for (int i = 0; i < 10; i++) + a.hello(); + return 0; +} diff --git a/gcc/testsuite/g++.dg/pph/x1ten-hellos.h b/gcc/testsuite/g++.dg/pph/x1ten-hellos.h new file mode 100644 index 0000000..2a53b66 --- /dev/null +++ b/gcc/testsuite/g++.dg/pph/x1ten-hellos.h @@ -0,0 +1,21 @@ +#ifndef A_H_ +#define A_H_ +#include <stdio.h> + +class A +{ +public: + A() { + x_ = 0; + printf ("constructing\n"); + } + + void hello(void) { + x_++; + printf ("Hello World (%d)\n", x_); + } + +private: + int x_; +}; +#endif diff --git a/gcc/testsuite/g++.dg/pph/pph.map b/gcc/testsuite/g++.dg/pph/pph.map index f0c7abd..2735af8 100644 --- a/gcc/testsuite/g++.dg/pph/pph.map +++ b/gcc/testsuite/g++.dg/pph/pph.map @@ -38,6 +47,7 @@ x1struct0.h x1struct0.pph x1struct1.h x1struct1.pph x1struct2.h x1struct2.pph x1template.h x1template.pph +x1ten-hellos.h x1ten-hellos.pph x1tmplclass.h x1tmplclass.pph x1tmplfunc.h x1tmplfunc.pph x1typerefs.h x1typerefs.pph