Patchwork [pph] New test (issue4629075)

login
register
mail settings
Submitter Diego Novillo
Date June 27, 2011, 4:39 p.m.
Message ID <20110627163937.6C7221DA195@topo.tor.corp.google.com>
Download mbox | patch
Permalink /patch/102219/
State New
Headers show

Comments

Diego Novillo - June 27, 2011, 4:39 p.m.
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


--
This patch is available for review at http://codereview.appspot.com/4629075
Gab Charette - June 27, 2011, 5:06 p.m.
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
>
Diego Novillo - June 27, 2011, 5:08 p.m.
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.
Lawrence Crowl - June 27, 2011, 6:56 p.m.
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).
Diego Novillo - June 27, 2011, 6:59 p.m.
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.

Patch

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