[PATCH (for next stage 1)] Add return type to gimple function dumps

[PATCH (for next stage 1)] Add return type to gimple function dumps

[David Malcolm]

[-- Attachment #1: Type: text/plain, Size: 934 bytes --]

Gimple function dumps contain the types of parameters, but not of the
return type.

The attached patch fixes this omission; here's an example of the
before/after diff:
$ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
--- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
+++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
@@ -1,3 +1,4 @@
+int
 ffff (int i)
 {
   int D.1731;


Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).

A couple of test cases needed tweaking, since they were counting the
number of occurrences of "int" in the gimple dump, which thus changed
for functions returning int (like the one above).

OK for next stage 1?

[motivation: am generating code in my JIT from other program's
representations, and have been debugging type mismatches in function
calls; the precise return types would otherwise have been non-obvious]

[-- Attachment #2: add-return-types-to-gimple-dumps.patch --]
[-- Type: text/x-patch, Size: 2338 bytes --]

commit cfef58ecb0d81e6b9c691217ad9efdce376fc0a5
Author: David Malcolm <dmalcolm@redhat.com>
Date:   Mon Mar 10 13:52:44 2014 -0400

    Dump the return type of functions in gimple dumps
    
    gcc/
    	* tree-cfg.c (dump_function_to_file): Dump the return type of
    	functions, in a line to itself before the function body, mimicking
    	the layout of a C function.
    
    gcc/testsuite/
    	* gcc.dg/tree-ssa/pr23401.c: Update the expected number of
    	occurrences of "int" in the gimple dump to reflect that the return
    	types of functions now show up in such dumps.
    	* gcc.dg/tree-ssa/pr27810.c: Likwise.

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c b/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
index 1d30ac7..3940692 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
@@ -19,6 +19,6 @@ int ffff(int i)
 
 /* We should not use extra temporaries apart from for i1 + i2.  */
 
-/* { dg-final { scan-tree-dump-times "int" 5 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "int" 6 "gimple" } } */
 /* { dg-final { scan-tree-dump-times "int D\\\." 1 "gimple" } } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c b/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
index c7da3bd..6d0904b 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
@@ -13,6 +13,6 @@ int qqq (int a)
 /* We should not use an extra temporary for the result of the
    function call.  */
 
-/* { dg-final { scan-tree-dump-times "int" 3 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "int" 4 "gimple" } } */
 /* { dg-final { scan-tree-dump-times "int D\\\." 1 "gimple" } } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 325285c..9335f4c 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7075,6 +7075,11 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
   struct function *fun = DECL_STRUCT_FUNCTION (fndecl);
 
   current_function_decl = fndecl;
+
+  /* Print the return type of the function: */
+  print_generic_expr (file, TREE_TYPE (TREE_TYPE (fun->decl)), dump_flags);
+  fprintf (file, "\n");
+
   fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : "");
 
   arg = DECL_ARGUMENTS (fndecl);

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[Richard Biener]

On Mon, Mar 10, 2014 at 8:22 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> Gimple function dumps contain the types of parameters, but not of the
> return type.
>
> The attached patch fixes this omission; here's an example of the
> before/after diff:
> $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> @@ -1,3 +1,4 @@
> +int
>  ffff (int i)
>  {
>    int D.1731;
>
>
> Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
>
> A couple of test cases needed tweaking, since they were counting the
> number of occurrences of "int" in the gimple dump, which thus changed
> for functions returning int (like the one above).
>
> OK for next stage 1?

ISTR doing that and giving up because of the sheer number of
testsuite FAILs this causes.  Did you properly test all languages
(I specifically remember Fortran here).

You also want to pass dump_flags | TDF_SLIM here otherwise
you'll get struct types expanded.

Richard.

> [motivation: am generating code in my JIT from other program's
> representations, and have been debugging type mismatches in function
> calls; the precise return types would otherwise have been non-obvious]

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[Jeff Law]

On 03/10/14 13:22, David Malcolm wrote:
> Gimple function dumps contain the types of parameters, but not of the
> return type.
>
> The attached patch fixes this omission; here's an example of the
> before/after diff:
> $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> @@ -1,3 +1,4 @@
> +int
>   ffff (int i)
>   {
>     int D.1731;
>
>
> Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
>
> A couple of test cases needed tweaking, since they were counting the
> number of occurrences of "int" in the gimple dump, which thus changed
> for functions returning int (like the one above).
>
> OK for next stage 1?
Conceptually OK.  As Richi notes, the work here is in fixing up the 
testsuite.  I didn't see a reply to Richi's question, particularly WRT 
the Fortran testsuite.

jeff

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[David Malcolm]

[-- Attachment #1: Type: text/plain, Size: 4817 bytes --]

On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
> On 03/10/14 13:22, David Malcolm wrote:
> > Gimple function dumps contain the types of parameters, but not of the
> > return type.
> >
> > The attached patch fixes this omission; here's an example of the
> > before/after diff:
> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> > @@ -1,3 +1,4 @@
> > +int
> >   ffff (int i)
> >   {
> >     int D.1731;
> >
> >
> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
> >
> > A couple of test cases needed tweaking, since they were counting the
> > number of occurrences of "int" in the gimple dump, which thus changed
> > for functions returning int (like the one above).
> >
> > OK for next stage 1?
> Conceptually OK.  As Richi notes, the work here is in fixing up the 
> testsuite.  I didn't see a reply to Richi's question, particularly WRT 
> the Fortran testsuite.

I'm attaching a revised version of the patch which adds the use of
TDF_SLIM (though it didn't appear to be necessary in the test I did of a
function returning a struct).

Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
using:
  --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto

I didn't see any new failures from this in the testsuite, in particular
gfortran.sum.  Here's a comparison of the before/after test results,
generated using my "jamais-vu" tool [1], with comments added by me
inline:

Comparing 16 common .sum files
------------------------------

 gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
 gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
 gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
 gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
 gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
 gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
 gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
 gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
 x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
 x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
 x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
 x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
 x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
 x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
 x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
 x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224

(...i.e. the totals were unchanged between unpatched/patched for all of
the .sum files; and yes, Fortran was tested.  Should there be a
gcj.sum?)

Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
----------------------------------------------------

 PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
 PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3

Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
----------------------------------------------

 PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
 PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4


(...my comparison tool isn't smart enough yet to tie these "went
away"/"appeared" results together; they reflect the fixups from the
patch).

Tests that went away in gcc/testsuite/go/go.sum: 2
--------------------------------------------------

 PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
 PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g

Tests appeared in gcc/testsuite/go/go.sum: 2
--------------------------------------------

 PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
 PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g

(...I hand edited the above, this main.go test embeds numerous paths,
which change between the two builds; so nothing really changed here).


Are the above results sane?

I'm not sure why I didn't see the failures Richi described; the patch
does appear to work (though again, should there be a gcj.sum? Did I miss
any frontends?)

OK for trunk?

Dave

[1] https://github.com/davidmalcolm/jamais-vu

[-- Attachment #2: Dump-the-return-type-of-functions-in-gimple-dumps.patch --]
[-- Type: text/x-patch, Size: 2561 bytes --]

From d89adb2ac085741e569d2988b3edf2bf6b481024 Mon Sep 17 00:00:00 2001
From: David Malcolm <dmalcolm@redhat.com>
Date: Mon, 10 Mar 2014 13:52:44 -0400
Subject: [PATCH] Dump the return type of functions in gimple dumps

gcc/
	* tree-cfg.c (dump_function_to_file): Dump the return type of
	functions, in a line to itself before the function body, mimicking
	the layout of a C function.

gcc/testsuite/
	* gcc.dg/tree-ssa/pr23401.c: Update the expected number of
	occurrences of "int" in the gimple dump to reflect that the return
	types of functions now show up in such dumps.
	* gcc.dg/tree-ssa/pr27810.c: Likwise.
---
 gcc/testsuite/gcc.dg/tree-ssa/pr23401.c | 2 +-
 gcc/testsuite/gcc.dg/tree-ssa/pr27810.c | 2 +-
 gcc/tree-cfg.c                          | 6 ++++++
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c b/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
index 1d30ac7..3940692 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr23401.c
@@ -19,6 +19,6 @@ int ffff(int i)
 
 /* We should not use extra temporaries apart from for i1 + i2.  */
 
-/* { dg-final { scan-tree-dump-times "int" 5 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "int" 6 "gimple" } } */
 /* { dg-final { scan-tree-dump-times "int D\\\." 1 "gimple" } } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c b/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
index c7da3bd..6d0904b 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr27810.c
@@ -13,6 +13,6 @@ int qqq (int a)
 /* We should not use an extra temporary for the result of the
    function call.  */
 
-/* { dg-final { scan-tree-dump-times "int" 3 "gimple" } } */
+/* { dg-final { scan-tree-dump-times "int" 4 "gimple" } } */
 /* { dg-final { scan-tree-dump-times "int D\\\." 1 "gimple" } } */
 /* { dg-final { cleanup-tree-dump "gimple" } } */
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 0fb2681..a5f09ea 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -7086,6 +7086,12 @@ dump_function_to_file (tree fndecl, FILE *file, int flags)
   struct function *fun = DECL_STRUCT_FUNCTION (fndecl);
 
   current_function_decl = fndecl;
+
+  /* Print the return type of the function: */
+  print_generic_expr (file, TREE_TYPE (TREE_TYPE (fun->decl)),
+		      dump_flags | TDF_SLIM);
+  fprintf (file, "\n");
+
   fprintf (file, "%s %s(", function_name (fun), tmclone ? "[tm-clone] " : "");
 
   arg = DECL_ARGUMENTS (fndecl);
-- 
1.8.5.3

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[Richard Biener]

On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
>> On 03/10/14 13:22, David Malcolm wrote:
>> > Gimple function dumps contain the types of parameters, but not of the
>> > return type.
>> >
>> > The attached patch fixes this omission; here's an example of the
>> > before/after diff:
>> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
>> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
>> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
>> > @@ -1,3 +1,4 @@
>> > +int
>> >   ffff (int i)
>> >   {
>> >     int D.1731;
>> >
>> >
>> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
>> >
>> > A couple of test cases needed tweaking, since they were counting the
>> > number of occurrences of "int" in the gimple dump, which thus changed
>> > for functions returning int (like the one above).
>> >
>> > OK for next stage 1?
>> Conceptually OK.  As Richi notes, the work here is in fixing up the
>> testsuite.  I didn't see a reply to Richi's question, particularly WRT
>> the Fortran testsuite.
>
> I'm attaching a revised version of the patch which adds the use of
> TDF_SLIM (though it didn't appear to be necessary in the test I did of a
> function returning a struct).
>
> Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
> using:
>   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
>
> I didn't see any new failures from this in the testsuite, in particular
> gfortran.sum.  Here's a comparison of the before/after test results,
> generated using my "jamais-vu" tool [1], with comments added by me
> inline:
>
> Comparing 16 common .sum files
> ------------------------------
>
>  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
>  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
>  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
>  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
>  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
>  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
>  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
>  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
>  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
>  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
>  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
>  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
>  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
>  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
>  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
>  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
>
> (...i.e. the totals were unchanged between unpatched/patched for all of
> the .sum files; and yes, Fortran was tested.  Should there be a
> gcj.sum?)
>
> Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
> ----------------------------------------------------
>
>  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
>  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
>
> Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
> ----------------------------------------------
>
>  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
>  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
>
>
> (...my comparison tool isn't smart enough yet to tie these "went
> away"/"appeared" results together; they reflect the fixups from the
> patch).
>
> Tests that went away in gcc/testsuite/go/go.sum: 2
> --------------------------------------------------
>
>  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>
> Tests appeared in gcc/testsuite/go/go.sum: 2
> --------------------------------------------
>
>  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>
> (...I hand edited the above, this main.go test embeds numerous paths,
> which change between the two builds; so nothing really changed here).
>
>
> Are the above results sane?

Yes.

> I'm not sure why I didn't see the failures Richi described; the patch
> does appear to work (though again, should there be a gcj.sum? Did I miss
> any frontends?)

Maybe I dumped

int foo (...

vs. your

int
foo (...

and that made the difference.

> OK for trunk?

Ok.

Thanks,
Richard.

> Dave
>
> [1] https://github.com/davidmalcolm/jamais-vu

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[David Malcolm]

On Tue, 2014-04-29 at 11:16 +0200, Richard Biener wrote:
> On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> > On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
> >> On 03/10/14 13:22, David Malcolm wrote:
> >> > Gimple function dumps contain the types of parameters, but not of the
> >> > return type.
> >> >
> >> > The attached patch fixes this omission; here's an example of the
> >> > before/after diff:
> >> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> >> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> >> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> >> > @@ -1,3 +1,4 @@
> >> > +int
> >> >   ffff (int i)
> >> >   {
> >> >     int D.1731;
> >> >
> >> >
> >> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
> >> >
> >> > A couple of test cases needed tweaking, since they were counting the
> >> > number of occurrences of "int" in the gimple dump, which thus changed
> >> > for functions returning int (like the one above).
> >> >
> >> > OK for next stage 1?
> >> Conceptually OK.  As Richi notes, the work here is in fixing up the
> >> testsuite.  I didn't see a reply to Richi's question, particularly WRT
> >> the Fortran testsuite.
> >
> > I'm attaching a revised version of the patch which adds the use of
> > TDF_SLIM (though it didn't appear to be necessary in the test I did of a
> > function returning a struct).
> >
> > Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
> > using:
> >   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
> >
> > I didn't see any new failures from this in the testsuite, in particular
> > gfortran.sum.  Here's a comparison of the before/after test results,
> > generated using my "jamais-vu" tool [1], with comments added by me
> > inline:
> >
> > Comparing 16 common .sum files
> > ------------------------------
> >
> >  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
> >  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
> >  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
> >  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
> >  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
> >  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
> >  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
> >  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
> >  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
> >  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
> >  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
> >  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
> >  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
> >  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
> >  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
> >  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
> >
> > (...i.e. the totals were unchanged between unpatched/patched for all of
> > the .sum files; and yes, Fortran was tested.  Should there be a
> > gcj.sum?)
> >
> > Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
> > ----------------------------------------------------
> >
> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
> >
> > Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
> > ----------------------------------------------
> >
> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
> >
> >
> > (...my comparison tool isn't smart enough yet to tie these "went
> > away"/"appeared" results together; they reflect the fixups from the
> > patch).
> >
> > Tests that went away in gcc/testsuite/go/go.sum: 2
> > --------------------------------------------------
> >
> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> >
> > Tests appeared in gcc/testsuite/go/go.sum: 2
> > --------------------------------------------
> >
> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> >
> > (...I hand edited the above, this main.go test embeds numerous paths,
> > which change between the two builds; so nothing really changed here).
> >
> >
> > Are the above results sane?
> 
> Yes.
> 
> > I'm not sure why I didn't see the failures Richi described; the patch
> > does appear to work (though again, should there be a gcj.sum? Did I miss
> > any frontends?)
> 
> Maybe I dumped
> 
> int foo (...
> 
> vs. your
> 
> int
> foo (...
> 
> and that made the difference.
> 
> > OK for trunk?
> 
> Ok.

Thanks; committed to trunk as r209902.

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[Richard Biener]

On Tue, Apr 29, 2014 at 5:01 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Tue, 2014-04-29 at 11:16 +0200, Richard Biener wrote:
>> On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
>> > On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
>> >> On 03/10/14 13:22, David Malcolm wrote:
>> >> > Gimple function dumps contain the types of parameters, but not of the
>> >> > return type.
>> >> >
>> >> > The attached patch fixes this omission; here's an example of the
>> >> > before/after diff:
>> >> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
>> >> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
>> >> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
>> >> > @@ -1,3 +1,4 @@
>> >> > +int
>> >> >   ffff (int i)
>> >> >   {
>> >> >     int D.1731;
>> >> >
>> >> >
>> >> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
>> >> >
>> >> > A couple of test cases needed tweaking, since they were counting the
>> >> > number of occurrences of "int" in the gimple dump, which thus changed
>> >> > for functions returning int (like the one above).
>> >> >
>> >> > OK for next stage 1?
>> >> Conceptually OK.  As Richi notes, the work here is in fixing up the
>> >> testsuite.  I didn't see a reply to Richi's question, particularly WRT
>> >> the Fortran testsuite.
>> >
>> > I'm attaching a revised version of the patch which adds the use of
>> > TDF_SLIM (though it didn't appear to be necessary in the test I did of a
>> > function returning a struct).
>> >
>> > Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
>> > using:
>> >   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
>> >
>> > I didn't see any new failures from this in the testsuite, in particular
>> > gfortran.sum.  Here's a comparison of the before/after test results,
>> > generated using my "jamais-vu" tool [1], with comments added by me
>> > inline:
>> >
>> > Comparing 16 common .sum files
>> > ------------------------------
>> >
>> >  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
>> >  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
>> >  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
>> >  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
>> >  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
>> >  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
>> >  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
>> >  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
>> >  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
>> >  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
>> >  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
>> >  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
>> >  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
>> >  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
>> >  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
>> >  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
>> >
>> > (...i.e. the totals were unchanged between unpatched/patched for all of
>> > the .sum files; and yes, Fortran was tested.  Should there be a
>> > gcj.sum?)
>> >
>> > Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
>> > ----------------------------------------------------
>> >
>> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
>> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
>> >
>> > Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
>> > ----------------------------------------------
>> >
>> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
>> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
>> >
>> >
>> > (...my comparison tool isn't smart enough yet to tie these "went
>> > away"/"appeared" results together; they reflect the fixups from the
>> > patch).
>> >
>> > Tests that went away in gcc/testsuite/go/go.sum: 2
>> > --------------------------------------------------
>> >
>> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>> >
>> > Tests appeared in gcc/testsuite/go/go.sum: 2
>> > --------------------------------------------
>> >
>> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>> >
>> > (...I hand edited the above, this main.go test embeds numerous paths,
>> > which change between the two builds; so nothing really changed here).
>> >
>> >
>> > Are the above results sane?
>>
>> Yes.
>>
>> > I'm not sure why I didn't see the failures Richi described; the patch
>> > does appear to work (though again, should there be a gcj.sum? Did I miss
>> > any frontends?)
>>
>> Maybe I dumped
>>
>> int foo (...
>>
>> vs. your
>>
>> int
>> foo (...
>>
>> and that made the difference.
>>
>> > OK for trunk?
>>
>> Ok.
>
> Thanks; committed to trunk as r209902.

Btw, I now see

int
int integer_zerop(const_tree) (const union tree_node * expr)
{
  union tree_node * D.86619;
  union tree_node * exp;

thus duplicated return type.

So - can you either revert or fix that?

Thanks,
Richard.

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[David Malcolm]

On Fri, 2014-05-16 at 14:59 +0200, Richard Biener wrote:
> On Tue, Apr 29, 2014 at 5:01 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > On Tue, 2014-04-29 at 11:16 +0200, Richard Biener wrote:
> >> On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> >> > On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
> >> >> On 03/10/14 13:22, David Malcolm wrote:
> >> >> > Gimple function dumps contain the types of parameters, but not of the
> >> >> > return type.
> >> >> >
> >> >> > The attached patch fixes this omission; here's an example of the
> >> >> > before/after diff:
> >> >> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> >> >> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> >> >> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> >> >> > @@ -1,3 +1,4 @@
> >> >> > +int
> >> >> >   ffff (int i)
> >> >> >   {
> >> >> >     int D.1731;
> >> >> >
> >> >> >
> >> >> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
> >> >> >
> >> >> > A couple of test cases needed tweaking, since they were counting the
> >> >> > number of occurrences of "int" in the gimple dump, which thus changed
> >> >> > for functions returning int (like the one above).
> >> >> >
> >> >> > OK for next stage 1?
> >> >> Conceptually OK.  As Richi notes, the work here is in fixing up the
> >> >> testsuite.  I didn't see a reply to Richi's question, particularly WRT
> >> >> the Fortran testsuite.
> >> >
> >> > I'm attaching a revised version of the patch which adds the use of
> >> > TDF_SLIM (though it didn't appear to be necessary in the test I did of a
> >> > function returning a struct).
> >> >
> >> > Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
> >> > using:
> >> >   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
> >> >
> >> > I didn't see any new failures from this in the testsuite, in particular
> >> > gfortran.sum.  Here's a comparison of the before/after test results,
> >> > generated using my "jamais-vu" tool [1], with comments added by me
> >> > inline:
> >> >
> >> > Comparing 16 common .sum files
> >> > ------------------------------
> >> >
> >> >  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
> >> >  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
> >> >  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
> >> >  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
> >> >  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
> >> >  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
> >> >  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
> >> >  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
> >> >  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
> >> >  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
> >> >  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
> >> >  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
> >> >  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
> >> >  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
> >> >  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
> >> >  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
> >> >
> >> > (...i.e. the totals were unchanged between unpatched/patched for all of
> >> > the .sum files; and yes, Fortran was tested.  Should there be a
> >> > gcj.sum?)
> >> >
> >> > Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
> >> > ----------------------------------------------------
> >> >
> >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
> >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
> >> >
> >> > Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
> >> > ----------------------------------------------
> >> >
> >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
> >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
> >> >
> >> >
> >> > (...my comparison tool isn't smart enough yet to tie these "went
> >> > away"/"appeared" results together; they reflect the fixups from the
> >> > patch).
> >> >
> >> > Tests that went away in gcc/testsuite/go/go.sum: 2
> >> > --------------------------------------------------
> >> >
> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> >> >
> >> > Tests appeared in gcc/testsuite/go/go.sum: 2
> >> > --------------------------------------------
> >> >
> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> >> >
> >> > (...I hand edited the above, this main.go test embeds numerous paths,
> >> > which change between the two builds; so nothing really changed here).
> >> >
> >> >
> >> > Are the above results sane?
> >>
> >> Yes.
> >>
> >> > I'm not sure why I didn't see the failures Richi described; the patch
> >> > does appear to work (though again, should there be a gcj.sum? Did I miss
> >> > any frontends?)
> >>
> >> Maybe I dumped
> >>
> >> int foo (...
> >>
> >> vs. your
> >>
> >> int
> >> foo (...
> >>
> >> and that made the difference.
> >>
> >> > OK for trunk?
> >>
> >> Ok.
> >
> > Thanks; committed to trunk as r209902.
> 
> Btw, I now see
> 
> int
> int integer_zerop(const_tree) (const union tree_node * expr)
> {
>   union tree_node * D.86619;
>   union tree_node * exp;
> 
> thus duplicated return type.
> 
> So - can you either revert or fix that?
> 
> Thanks,
> Richard.

Sorry about this.

I dug into the decl-related dump output:

  int
  int integer_zerop(const_tree) (const union tree_node * expr)

to see where each component comes from:

  int
  ^^^ from r209902
  int integer_zerop(const_tree) (const union tree_node * expr)
  ^^^^^^^function_name ()^^^^^^ ^^^dump_function_to_file ()^^^

Clearly I didn't test enough with C++; what I committed works with C,
but function_name calls fndecl_name which calls
lang_hooks.decl_printable_name with verbosity 2.

The C++ langhook for decl_printable_name includes the return type at
v=2, whereas the default lhd_decl_printable_name merely returns
  IDENTIFIER_POINTER (DECL_NAME (decl))

hence the duplicated return type for C++ (and possibly java), and single
copy of the return type for C.

Note also that the C++ langhook at v=2 also includes the parameter
types, which is why the C++ dump contains the param types twice; i.e.:

  int
  int integer_zerop(const_tree) (const union tree_node * expr)
                   ^^^^types^^^ ^^^^^^types and names^^^^^^^^^

Is the duplication of the parameter signature regarded as an issue [1],
or expected behavior given that language's support for overloading?  (I
see this duplication with e.g. 4.8, fwiw).

Dave

[1] if so, perhaps some kind of tweaking of the verbosity used when
calling decl_printable_name from dump_function_to_file could fix this?
though I imagine the redundant info can make debugging easier

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[David Malcolm]

On Fri, 2014-05-16 at 12:17 -0400, David Malcolm wrote:
> On Fri, 2014-05-16 at 14:59 +0200, Richard Biener wrote:
> > On Tue, Apr 29, 2014 at 5:01 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> > > On Tue, 2014-04-29 at 11:16 +0200, Richard Biener wrote:
> > >> On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
> > >> > On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
> > >> >> On 03/10/14 13:22, David Malcolm wrote:
> > >> >> > Gimple function dumps contain the types of parameters, but not of the
> > >> >> > return type.
> > >> >> >
> > >> >> > The attached patch fixes this omission; here's an example of the
> > >> >> > before/after diff:
> > >> >> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
> > >> >> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
> > >> >> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
> > >> >> > @@ -1,3 +1,4 @@
> > >> >> > +int
> > >> >> >   ffff (int i)
> > >> >> >   {
> > >> >> >     int D.1731;
> > >> >> >
> > >> >> >
> > >> >> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
> > >> >> >
> > >> >> > A couple of test cases needed tweaking, since they were counting the
> > >> >> > number of occurrences of "int" in the gimple dump, which thus changed
> > >> >> > for functions returning int (like the one above).
> > >> >> >
> > >> >> > OK for next stage 1?
> > >> >> Conceptually OK.  As Richi notes, the work here is in fixing up the
> > >> >> testsuite.  I didn't see a reply to Richi's question, particularly WRT
> > >> >> the Fortran testsuite.
> > >> >
> > >> > I'm attaching a revised version of the patch which adds the use of
> > >> > TDF_SLIM (though it didn't appear to be necessary in the test I did of a
> > >> > function returning a struct).
> > >> >
> > >> > Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
> > >> > using:
> > >> >   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
> > >> >
> > >> > I didn't see any new failures from this in the testsuite, in particular
> > >> > gfortran.sum.  Here's a comparison of the before/after test results,
> > >> > generated using my "jamais-vu" tool [1], with comments added by me
> > >> > inline:
> > >> >
> > >> > Comparing 16 common .sum files
> > >> > ------------------------------
> > >> >
> > >> >  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
> > >> >  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
> > >> >  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
> > >> >  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
> > >> >  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
> > >> >  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
> > >> >  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
> > >> >  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
> > >> >  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
> > >> >  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
> > >> >  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
> > >> >  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
> > >> >  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
> > >> >  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
> > >> >  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
> > >> >  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
> > >> >
> > >> > (...i.e. the totals were unchanged between unpatched/patched for all of
> > >> > the .sum files; and yes, Fortran was tested.  Should there be a
> > >> > gcj.sum?)
> > >> >
> > >> > Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
> > >> > ----------------------------------------------------
> > >> >
> > >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
> > >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
> > >> >
> > >> > Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
> > >> > ----------------------------------------------
> > >> >
> > >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
> > >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
> > >> >
> > >> >
> > >> > (...my comparison tool isn't smart enough yet to tie these "went
> > >> > away"/"appeared" results together; they reflect the fixups from the
> > >> > patch).
> > >> >
> > >> > Tests that went away in gcc/testsuite/go/go.sum: 2
> > >> > --------------------------------------------------
> > >> >
> > >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> > >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> > >> >
> > >> > Tests appeared in gcc/testsuite/go/go.sum: 2
> > >> > --------------------------------------------
> > >> >
> > >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
> > >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
> > >> >
> > >> > (...I hand edited the above, this main.go test embeds numerous paths,
> > >> > which change between the two builds; so nothing really changed here).
> > >> >
> > >> >
> > >> > Are the above results sane?
> > >>
> > >> Yes.
> > >>
> > >> > I'm not sure why I didn't see the failures Richi described; the patch
> > >> > does appear to work (though again, should there be a gcj.sum? Did I miss
> > >> > any frontends?)
> > >>
> > >> Maybe I dumped
> > >>
> > >> int foo (...
> > >>
> > >> vs. your
> > >>
> > >> int
> > >> foo (...
> > >>
> > >> and that made the difference.
> > >>
> > >> > OK for trunk?
> > >>
> > >> Ok.
> > >
> > > Thanks; committed to trunk as r209902.
> > 
> > Btw, I now see
> > 
> > int
> > int integer_zerop(const_tree) (const union tree_node * expr)
> > {
> >   union tree_node * D.86619;
> >   union tree_node * exp;
> > 
> > thus duplicated return type.
> > 
> > So - can you either revert or fix that?
> > 
> > Thanks,
> > Richard.
> 
> Sorry about this.
> 
> I dug into the decl-related dump output:
> 
>   int
>   int integer_zerop(const_tree) (const union tree_node * expr)
> 
> to see where each component comes from:
> 
>   int
>   ^^^ from r209902
>   int integer_zerop(const_tree) (const union tree_node * expr)
>   ^^^^^^^function_name ()^^^^^^ ^^^dump_function_to_file ()^^^
> 
> Clearly I didn't test enough with C++; what I committed works with C,
> but function_name calls fndecl_name which calls
> lang_hooks.decl_printable_name with verbosity 2.
> 
> The C++ langhook for decl_printable_name includes the return type at
> v=2, whereas the default lhd_decl_printable_name merely returns
>   IDENTIFIER_POINTER (DECL_NAME (decl))
> 
> hence the duplicated return type for C++ (and possibly java), and single
> copy of the return type for C.
> 
> Note also that the C++ langhook at v=2 also includes the parameter
> types, which is why the C++ dump contains the param types twice; i.e.:
> 
>   int
>   int integer_zerop(const_tree) (const union tree_node * expr)
>                    ^^^^types^^^ ^^^^^^types and names^^^^^^^^^
> 
> Is the duplication of the parameter signature regarded as an issue [1],
> or expected behavior given that language's support for overloading?  (I
> see this duplication with e.g. 4.8, fwiw).

In the meantime, I've reverted this, as r210533.

> Dave
> 
> [1] if so, perhaps some kind of tweaking of the verbosity used when
> calling decl_printable_name from dump_function_to_file could fix this?
> though I imagine the redundant info can make debugging easier

Re: [PATCH (for next stage 1)] Add return type to gimple function dumps

[Richard Biener]

On Fri, May 16, 2014 at 6:17 PM, David Malcolm <dmalcolm@redhat.com> wrote:
> On Fri, 2014-05-16 at 14:59 +0200, Richard Biener wrote:
>> On Tue, Apr 29, 2014 at 5:01 PM, David Malcolm <dmalcolm@redhat.com> wrote:
>> > On Tue, 2014-04-29 at 11:16 +0200, Richard Biener wrote:
>> >> On Tue, Apr 29, 2014 at 2:58 AM, David Malcolm <dmalcolm@redhat.com> wrote:
>> >> > On Thu, 2014-04-24 at 15:46 -0600, Jeff Law wrote:
>> >> >> On 03/10/14 13:22, David Malcolm wrote:
>> >> >> > Gimple function dumps contain the types of parameters, but not of the
>> >> >> > return type.
>> >> >> >
>> >> >> > The attached patch fixes this omission; here's an example of the
>> >> >> > before/after diff:
>> >> >> > $ diff -up /tmp/pr23401.c.004t.gimple.old /tmp/pr23401.c.004t.gimple.new
>> >> >> > --- /tmp/pr23401.c.004t.gimple.old      2014-03-10 13:40:08.972063541 -0400
>> >> >> > +++ /tmp/pr23401.c.004t.gimple.new      2014-03-10 13:39:49.346515464 -0400
>> >> >> > @@ -1,3 +1,4 @@
>> >> >> > +int
>> >> >> >   ffff (int i)
>> >> >> >   {
>> >> >> >     int D.1731;
>> >> >> >
>> >> >> >
>> >> >> > Successfully bootstrapped and regrtested on x86_64 Linux (Fedora 20).
>> >> >> >
>> >> >> > A couple of test cases needed tweaking, since they were counting the
>> >> >> > number of occurrences of "int" in the gimple dump, which thus changed
>> >> >> > for functions returning int (like the one above).
>> >> >> >
>> >> >> > OK for next stage 1?
>> >> >> Conceptually OK.  As Richi notes, the work here is in fixing up the
>> >> >> testsuite.  I didn't see a reply to Richi's question, particularly WRT
>> >> >> the Fortran testsuite.
>> >> >
>> >> > I'm attaching a revised version of the patch which adds the use of
>> >> > TDF_SLIM (though it didn't appear to be necessary in the test I did of a
>> >> > function returning a struct).
>> >> >
>> >> > Successfully bootstrapped & regrtested on x86_64 Linux (Fedora 20),
>> >> > using:
>> >> >   --enable-languages=c,c++,objc,obj-c++,java,fortran,ada,go,lto
>> >> >
>> >> > I didn't see any new failures from this in the testsuite, in particular
>> >> > gfortran.sum.  Here's a comparison of the before/after test results,
>> >> > generated using my "jamais-vu" tool [1], with comments added by me
>> >> > inline:
>> >> >
>> >> > Comparing 16 common .sum files
>> >> > ------------------------------
>> >> >
>> >> >  gcc/testsuite/ada/acats/acats.sum : total: 2320 PASS: 2320
>> >> >  gcc/testsuite/g++/g++.sum : total: 90421 FAIL: 3 PASS: 86969 XFAIL: 445 UNSUPPORTED: 3004
>> >> >  gcc/testsuite/gcc/gcc.sum : total: 110458 FAIL: 45 PASS: 108292 XFAIL: 265 XPASS: 33 UNSUPPORTED: 1823
>> >> >  gcc/testsuite/gfortran/gfortran.sum : total: 45717 PASS: 45600 XFAIL: 52 UNSUPPORTED: 65
>> >> >  gcc/testsuite/gnat/gnat.sum : total: 1255 PASS: 1234 XFAIL: 18 UNSUPPORTED: 3
>> >> >  gcc/testsuite/go/go.sum : total: 7266 PASS: 7258 XFAIL: 1 UNTESTED: 6 UNSUPPORTED: 1
>> >> >  gcc/testsuite/obj-c++/obj-c++.sum : total: 1450 PASS: 1354 XFAIL: 10 UNSUPPORTED: 86
>> >> >  gcc/testsuite/objc/objc.sum : total: 2973 PASS: 2893 XFAIL: 6 UNSUPPORTED: 74
>> >> >  x86_64-unknown-linux-gnu/boehm-gc/testsuite/boehm-gc.sum : total: 13 PASS: 12 UNSUPPORTED: 1
>> >> >  x86_64-unknown-linux-gnu/libatomic/testsuite/libatomic.sum : total: 54 PASS: 54
>> >> >  x86_64-unknown-linux-gnu/libffi/testsuite/libffi.sum : total: 1856 PASS: 1801 UNSUPPORTED: 55
>> >> >  x86_64-unknown-linux-gnu/libgo/libgo.sum : total: 122 PASS: 122
>> >> >  x86_64-unknown-linux-gnu/libgomp/testsuite/libgomp.sum : total: 2420 PASS: 2420
>> >> >  x86_64-unknown-linux-gnu/libitm/testsuite/libitm.sum : total: 30 PASS: 26 XFAIL: 3 UNSUPPORTED: 1
>> >> >  x86_64-unknown-linux-gnu/libjava/testsuite/libjava.sum : total: 2586 PASS: 2582 XFAIL: 4
>> >> >  x86_64-unknown-linux-gnu/libstdc++-v3/testsuite/libstdc++.sum : total: 10265 PASS: 10000 XFAIL: 41 UNSUPPORTED: 224
>> >> >
>> >> > (...i.e. the totals were unchanged between unpatched/patched for all of
>> >> > the .sum files; and yes, Fortran was tested.  Should there be a
>> >> > gcj.sum?)
>> >> >
>> >> > Tests that went away in gcc/testsuite/gcc/gcc.sum: 2
>> >> > ----------------------------------------------------
>> >> >
>> >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 5
>> >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 3
>> >> >
>> >> > Tests appeared in gcc/testsuite/gcc/gcc.sum: 2
>> >> > ----------------------------------------------
>> >> >
>> >> >  PASS: gcc.dg/tree-ssa/pr23401.c scan-tree-dump-times gimple "int" 6
>> >> >  PASS: gcc.dg/tree-ssa/pr27810.c scan-tree-dump-times gimple "int" 4
>> >> >
>> >> >
>> >> > (...my comparison tool isn't smart enough yet to tie these "went
>> >> > away"/"appeared" results together; they reflect the fixups from the
>> >> > patch).
>> >> >
>> >> > Tests that went away in gcc/testsuite/go/go.sum: 2
>> >> > --------------------------------------------------
>> >> >
>> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>> >> >
>> >> > Tests appeared in gcc/testsuite/go/go.sum: 2
>> >> > --------------------------------------------
>> >> >
>> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) compilation,  -O2 -g
>> >> >  PASS: go.test/test/dwarf/dwarf.dir/main.go (lots of refs to path of build) execution,  -O2 -g
>> >> >
>> >> > (...I hand edited the above, this main.go test embeds numerous paths,
>> >> > which change between the two builds; so nothing really changed here).
>> >> >
>> >> >
>> >> > Are the above results sane?
>> >>
>> >> Yes.
>> >>
>> >> > I'm not sure why I didn't see the failures Richi described; the patch
>> >> > does appear to work (though again, should there be a gcj.sum? Did I miss
>> >> > any frontends?)
>> >>
>> >> Maybe I dumped
>> >>
>> >> int foo (...
>> >>
>> >> vs. your
>> >>
>> >> int
>> >> foo (...
>> >>
>> >> and that made the difference.
>> >>
>> >> > OK for trunk?
>> >>
>> >> Ok.
>> >
>> > Thanks; committed to trunk as r209902.
>>
>> Btw, I now see
>>
>> int
>> int integer_zerop(const_tree) (const union tree_node * expr)
>> {
>>   union tree_node * D.86619;
>>   union tree_node * exp;
>>
>> thus duplicated return type.
>>
>> So - can you either revert or fix that?
>>
>> Thanks,
>> Richard.
>
> Sorry about this.
>
> I dug into the decl-related dump output:
>
>   int
>   int integer_zerop(const_tree) (const union tree_node * expr)
>
> to see where each component comes from:
>
>   int
>   ^^^ from r209902
>   int integer_zerop(const_tree) (const union tree_node * expr)
>   ^^^^^^^function_name ()^^^^^^ ^^^dump_function_to_file ()^^^
>
> Clearly I didn't test enough with C++; what I committed works with C,
> but function_name calls fndecl_name which calls
> lang_hooks.decl_printable_name with verbosity 2.

Using a langhook here is to get an unmangled name

> The C++ langhook for decl_printable_name includes the return type at
> v=2, whereas the default lhd_decl_printable_name merely returns
>   IDENTIFIER_POINTER (DECL_NAME (decl))
>
> hence the duplicated return type for C++ (and possibly java), and single
> copy of the return type for C.

Hmm, yeah.  I suppose the C++ variant includes argument types
because that's encoded in mangling.  You might want to check
C++ and behavior on extern "C" int foo (int) {}

> Note also that the C++ langhook at v=2 also includes the parameter
> types, which is why the C++ dump contains the param types twice; i.e.:
>
>   int
>   int integer_zerop(const_tree) (const union tree_node * expr)
>                    ^^^^types^^^ ^^^^^^types and names^^^^^^^^^
>
> Is the duplication of the parameter signature regarded as an issue [1],
> or expected behavior given that language's support for overloading?  (I
> see this duplication with e.g. 4.8, fwiw).

As fndecl_name already calls a langhook we might do different things
dependent on lang_hooks.name ... (ugh).

Note that handling arguments in the langhook is probably good (and
maybe the default implementation should do that as well), as you
get fancy dumping of template parameters.

Richard.

> Dave
>
> [1] if so, perhaps some kind of tweaking of the verbosity used when
> calling decl_printable_name from dump_function_to_file could fix this?
> though I imagine the redundant info can make debugging easier
>