On 08/06/2014 19:43, Roman Gareev wrote:
Hi Tobias,
This file is empty. It seems to be the perfect place for gloog_isl,
maybe give it a more descriptive name. E.g.,
graphite_regenerate_ast_isl()
We could then rename gloog, to graphite_regenerate_ast_cloog().
gloog comes from graphite + cloog and does not make sense in the context
of isl.
Would it be better to rename gloog in a separate patch (because it
could cause renaming of gloog_error, for eaxmple, and increase the
size of the patch)?
Yes, you are right. Please make this a separate patch and rebase this
patch on the gloog patch.
I tried to incorporate all your comments in the following patch. I
also attach the change log to this message.
Comments inline.
#endif /* ! GCC_FLAG_TYPES_H */
diff --git a/gcc/graphite-clast-to-gimple.h b/gcc/graphite-clast-to-gimple.h
index fc5a679..5e23d94 100644
--- a/gcc/graphite-clast-to-gimple.h
+++ b/gcc/graphite-clast-to-gimple.h
@@ -21,6 +21,8 @@ along with GCC; see the file COPYING3. If not see
#ifndef GCC_GRAPHITE_CLAST_TO_GIMPLE_H
#define GCC_GRAPHITE_CLAST_TO_GIMPLE_H
+#include "graphite-htab.h"
+
extern CloogState *cloog_state;
/* Data structure for CLooG program representation. */
@@ -30,14 +32,7 @@ struct cloog_prog_clast {
struct clast_stmt *stmt;
};
-/* Stores BB's related PBB. */
-
-struct bb_pbb_def
-{
- basic_block bb;
- poly_bb_p pbb;
-};
-
+extern bool gloog (scop_p, bb_pbb_htab_type);
extern void debug_clast_stmt (struct clast_stmt *);
extern void print_clast_stmt (FILE *, struct clast_stmt *);
diff --git a/gcc/graphite-htab.h b/gcc/graphite-htab.h
index d67dd0c..9f31fac 100644
--- a/gcc/graphite-htab.h
+++ b/gcc/graphite-htab.h
@@ -22,7 +22,14 @@ along with GCC; see the file COPYING3. If not see
#define GCC_GRAPHITE_HTAB_H
#include "hash-table.h"
-#include "graphite-clast-to-gimple.h"
+
+/* Stores BB's related PBB. */
+
+struct bb_pbb_def
+{
+ basic_block bb;
+ poly_bb_p pbb;
+};
/* Hashtable helpers. */
@@ -52,7 +59,6 @@ bb_pbb_hasher::equal (const value_type *bp1, const
compare_type *bp2)
typedef hash_table <bb_pbb_hasher> bb_pbb_htab_type;
-extern bool gloog (scop_p, bb_pbb_htab_type);
poly_bb_p find_pbb_via_hash (bb_pbb_htab_type, basic_block);
bool loop_is_parallel_p (loop_p, bb_pbb_htab_type, int);
scop_p get_loop_body_pbbs (loop_p, bb_pbb_htab_type, vec<poly_bb_p> *);
I assume all this could be part of the gloog patch?
diff --git a/gcc/graphite-isl-ast-to-gimple.c b/gcc/graphite-isl-ast-to-gimple.c
new file mode 100644
index 0000000..309ba28
--- /dev/null
+++ b/gcc/graphite-isl-ast-to-gimple.c
@@ -0,0 +1,132 @@
+/* Translation of ISL AST to Gimple.
+ Copyright (C) 2014 Free Software Foundation, Inc.
+ Contributed by Roman Gareev <gareevro...@gmail.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify
+it under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful,
+but WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+GNU General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+#include "config.h"
+
+#include <isl/set.h>
+#include <isl/map.h>
+#include <isl/union_map.h>
+#include <isl/ast_build.h>
+
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "basic-block.h"
+#include "tree-ssa-alias.h"
+#include "internal-fn.h"
+#include "gimple-expr.h"
+#include "is-a.h"
+#include "gimple.h"
+#include "gimple-iterator.h"
+#include "tree-ssa-loop.h"
+#include "tree-pass.h"
+#include "cfgloop.h"
+#include "tree-data-ref.h"
+#include "sese.h"
I am surprised. Are all these includes really needed to get _this_ patch
compile? (I asked this before).
diff --git a/gcc/testsuite/gcc.dg/graphite/scop-23.c
b/gcc/testsuite/gcc.dg/graphite/scop-23.c
new file mode 100644
index 0000000..868222a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/scop-23.c
Maybe a more descriptive name:
isl-codegen-loop-dumping.c
@@ -0,0 +1,32 @@
+/* { dg-additional-options "-floop-parallelize-all -O2
-fgraphite-code-generator=isl" } */
Why do we need -floop-parallelize-all? -fgraphite-identity should be
enough.
+#include <stdio.h>
+
+#define N 16
No need for these defines.
+
+int
+main1 (int n, int *a)
+{
+ int i, j;
+
+ for (i = 0; i < n - 1; i++)
+ for (j = 0; j < n; j++)
+ a[j] = i + n;
And just a single loop.
+
+ for (j = 0; j < n; j++)
+ if (a[j] != i + n - 1)
+ __builtin_abort ();
No need for the check.
+int
+main ()
+{
+ int a[N];
+ main1 (N, a);
+ return 0;
+}
No need for main.
And we have a minimal test case.
+/* { dg-final { scan-tree-dump-times "ISL AST generated by ISL: \nfor \\(int c1 = 0; c1 < n - 1;
c1 \\+= 1\\)\n for \\(int c3 = 0; c3 < n; c3 \\+= 1\\)\n S_5\\(c1, c3\\);" 1
"graphite"} } */
This is ugly, but there is little we can do about it. Maybe you can ask
on the mailing list if there is a way to write this in multiple lines?
On possibility might be that you write:
+/* { dg-final { scan-tree-dump-times "ISL AST generated by ISL: \n
? for \\(int c1 = 0; c1 < n - 1; c1 \\+= 1\\)\n
? for \\(int c3 = 0; c3 < n; c3 \\+= 1\\)\n
? S_5\\(c1, c3\\);" 1 "graphite"}
? } */
The idea is that you add newlines, but the question mark that is
following immediately after makes them again optional in the regexp.
I would spend maybe 30-45 minutes on this. If you don't find any
solution, let's live with it.
Cheers,
Tobias
