This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [GSoC] How to get started with the isl code generation


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 <gareevroman@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]