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
>From d89adb2ac085741e569d2988b3edf2bf6b481024 Mon Sep 17 00:00:00 2001
From: David Malcolm <[email protected]>
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
