Re: [GSoC] generation of Gimple loops with empty bodies
> Surprising. Is the symbol defined in libisl.so? You could use objdump to
> check?
If I am not mistaken, objdump shows that libisl.so contains it.
objdump -d libisl.so.10.2.2 | grep isl_ast_expr_get_val
00060380 :
60383: 74 4b je 603d0
60389: 75 0d jne60398 Index: gcc/graphite-isl-ast-to-gimple.c
===
--- gcc/graphite-isl-ast-to-gimple.c(revision 212294)
+++ gcc/graphite-isl-ast-to-gimple.c(working copy)
@@ -42,16 +42,549 @@
#include "cfgloop.h"
#include "tree-data-ref.h"
#include "sese.h"
+#include "tree-ssa-loop-manip.h"
+#include "tree-scalar-evolution.h"
+#include
#ifdef HAVE_cloog
#include "graphite-poly.h"
#include "graphite-isl-ast-to-gimple.h"
+#include "graphite-htab.h"
/* This flag is set when an error occurred during the translation of
ISL AST to Gimple. */
static bool graphite_regenerate_error;
+/* We always use signed 128, until isl is able to give information about
+types */
+
+static tree *graphite_expression_size_type = &int128_integer_type_node;
+
+/* Converts a GMP constant VAL to a tree and returns it. */
+
+static tree
+gmp_cst_to_tree (tree type, mpz_t val)
+{
+ tree t = type ? type : integer_type_node;
+ mpz_t tmp;
+
+ mpz_init (tmp);
+ mpz_set (tmp, val);
+ wide_int wi = wi::from_mpz (t, tmp, true);
+ mpz_clear (tmp);
+
+ return wide_int_to_tree (t, wi);
+}
+
+/* Verifies properties that GRAPHITE should maintain during translation. */
+
+static inline void
+graphite_verify (void)
+{
+#ifdef ENABLE_CHECKING
+ verify_loop_structure ();
+ verify_loop_closed_ssa (true);
+#endif
+}
+
+/* The comparison function for ivs_params_map */
+
+struct id_comp
+{
+ bool operator()(isl_id *id1, isl_id *id2) const
+ {
+const char *name1 = isl_id_get_name (id1);
+const char *name2 = isl_id_get_name (id2);
+return strcmp (name1, name2) == 0;
+ }
+};
+
+/* IVS_PARAMS_MAP maps ISL's scattering and parameter identifiers
+ to corresponding trees. */
+
+typedef struct ivs_params {
+ std::map ivs_params_map;
+ sese region;
+} *ivs_params_p;
+
+/* Free all memory allocated for ISL's identifiers. */
+
+void ivs_params_clear (ivs_params_p ip)
+{
+ std::map::iterator it;
+ for (it = ip->ivs_params_map.begin (); it != ip->ivs_params_map.end (); it++)
+{
+ isl_id_free (it->first);
+}
+}
+
+static tree
+gcc_expression_from_isl_expression (tree type, __isl_keep isl_ast_expr *,
+ ivs_params_p ip);
+
+/* Returns the tree variable from the name of isl_id, which is stored
+ in the isl_ast_expr EXPR_ID that was given in ISL representation. */
+
+static tree
+gcc_expression_from_isl_ast_expr_id (__isl_keep isl_ast_expr *expr_id,
+ivs_params_p ip)
+{
+ gcc_assert (isl_ast_expr_get_type (expr_id) == isl_ast_expr_id);
+ isl_id *tmp_isl_id = isl_ast_expr_get_id (expr_id);
+ std::map::iterator res;
+ res = ip->ivs_params_map.find (tmp_isl_id);
+ isl_id_free (tmp_isl_id);
+ gcc_assert (res != ip->ivs_params_map.end ());
+ return res->second;
+}
+
+/* Converts a isl_ast_expr_int expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+gcc_expression_from_isl_expr_int (tree type, __isl_keep isl_ast_expr *expr)
+{
+ gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_int);
+ isl_int val;
+ isl_int_init (val);
+ if (isl_ast_expr_get_int (expr, &val) == -1)
+{
+ isl_int_clear (val);
+ return NULL_TREE;
+}
+ else
+return gmp_cst_to_tree (type, val);
+}
+
+/* Converts a binary isl_ast_expr_op expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+binary_op_to_tree (tree type, __isl_keep isl_ast_expr *expr, ivs_params_p ip)
+{
+ isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
+ tree tree_lhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
+ isl_ast_expr_free (arg_expr);
+ arg_expr = isl_ast_expr_get_op_arg (expr, 1);
+ tree tree_rhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
+ isl_ast_expr_free (arg_expr);
+ switch (isl_ast_expr_get_op_type (expr))
+{
+case isl_ast_op_add:
+ return fold_build2 (PLUS_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_sub:
+ return fold_build2 (MINUS_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_mul:
+ return fold_build2 (MULT_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_div:
+ return fold_build2 (EXACT_DIV_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_fdiv_q:
+ return fold_build2 (FLOOR_DIV_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_and:
+ return fold_build2 (TRUTH_ANDIF_EXPR, type,
+ tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_or:
+ return fold_build2 (TRUTH_ORIF_EXPR, type, tree_lhs_expr, tree_rhs_expr);
+
+case isl_ast_op_e
Re: [GSoC] generation of Gimple loops with empty bodies
On 08/07/2014 14:47, Roman Gareev wrote:
Surprising. Is the symbol defined in libisl.so? You could use objdump to
>check?
If I am not mistaken, objdump shows that libisl.so contains it.
objdump -d libisl.so.10.2.2 | grep isl_ast_expr_get_val
00060380 :
60383: 74 4b je 603d0
60389: 75 0d jne60398
I would use -x to see the headers. This should tell you if it is defined
or only used there.
+#include "graphite-htab.h"
Is this still needed?
(Just a question, no need to act on it if it is still needed)
+struct id_comp
+{
+ bool operator()(isl_id *id1, isl_id *id2) const
+ {
+const char *name1 = isl_id_get_name (id1);
+const char *name2 = isl_id_get_name (id2);
+return strcmp (name1, name2) == 0;
+ }
+};
No need to provide this. Comparing by pointer should be fine.
+
+/* IVS_PARAMS_MAP maps ISL's scattering and parameter identifiers
+ to corresponding trees. */
+typedef struct ivs_params {
+ std::map ivs_params_map;
What about calling it isl_id_to_tree?
+ sese region;
Is this region needed?
+static tree
+gcc_expression_from_isl_expression (tree type, __isl_keep isl_ast_expr *,
+ ivs_params_p ip);
I would make those functions __isl_take the expression.
+
+/* Returns the tree variable from the name of isl_id, which is stored
+ in the isl_ast_expr EXPR_ID that was given in ISL representation. */
This takes a couple of turns. What about:
"Return the tree variable that corresponds to the given isl ast identifier
expression (an isl_ast_expr of type isl_ast_expr_id)."
+
+static tree
+gcc_expression_from_isl_ast_expr_id (__isl_keep isl_ast_expr *expr_id,
+ivs_params_p ip)
+{
+ gcc_assert (isl_ast_expr_get_type (expr_id) == isl_ast_expr_id);
+ isl_id *tmp_isl_id = isl_ast_expr_get_id (expr_id);
+ std::map::iterator res;
+ res = ip->ivs_params_map.find (tmp_isl_id);
+ isl_id_free (tmp_isl_id);
+ gcc_assert (res != ip->ivs_params_map.end ());
Maybe add an assert message ala "Could not map isl_id to tree expression"?
+ return res->second;
+}
+
+/* Converts a isl_ast_expr_int expression E to a GCC expression tree of
Converts _aN_ isl_ast_expr_int ...
+ type TYPE. */
+
+static tree
+gcc_expression_from_isl_expr_int (tree type, __isl_keep isl_ast_expr *expr)
+{
+ gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_int);
+ isl_int val;
+ isl_int_init (val);
+ if (isl_ast_expr_get_int (expr, &val) == -1)
+{
+ isl_int_clear (val);
Sorry, but this still needs to be debugged. :-( I am too busy to do it
myself, so i am afraid you may just need to dig into it yourself. :-(
+/* Converts a binary isl_ast_expr_op expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+binary_op_to_tree (tree type, __isl_keep isl_ast_expr *expr, ivs_params_p ip)
+{
+ isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
+ tree tree_lhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
+ isl_ast_expr_free (arg_expr);
If the from_isl_expression functions become __isl_take this free becomes
unnecessary.
+/* Converts a isl_ast_expr_op expression E with unknown number of arguments
Converts _an_ isl
+/* Converts an isl_ast_expr_op expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+gcc_expression_from_isl_expr_op (tree type, __isl_keep isl_ast_expr *expr,
+ivs_params_p ip)
+{
+ gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_op);
+ switch (isl_ast_expr_get_op_type (expr))
+{
+/* These isl ast expressions are not supported yet. */
+case isl_ast_op_error:
+case isl_ast_op_call:
+case isl_ast_op_and_then:
+case isl_ast_op_or_else:
+case isl_ast_op_pdiv_q:
+case isl_ast_op_pdiv_r:
+case isl_ast_op_select:
+ gcc_unreachable ();
+
+case isl_ast_op_max:
+case isl_ast_op_min:
+ return nary_op_to_tree (type, expr, ip);
+
+case isl_ast_op_add:
+case isl_ast_op_sub:
+case isl_ast_op_mul:
+case isl_ast_op_div:
+case isl_ast_op_fdiv_q:
+case isl_ast_op_and:
+case isl_ast_op_or:
+case isl_ast_op_eq:
+case isl_ast_op_le:
+case isl_ast_op_lt:
+case isl_ast_op_ge:
+case isl_ast_op_gt:
+ return binary_op_to_tree (type, expr, ip);
+
+case isl_ast_op_minus:
+ return unary_op_to_tree (type, expr, ip);
+
+case isl_ast_op_cond:
+ return ternary_op_to_tree (type, expr, ip);
+
+default:
+ gcc_unreachable ();
+}
+
+ return NULL_TREE;
+}
+
+/* Converts a ISL AST expression E back to a GCC expression tree of
_an_
+/* We use this function to get the upper bound because of the form,
+ which is used by isl to represent loops:
+
+ for (iterator = init; cond; iterator += inc)
+
+ {
+
+ ...
+
+ }
+
+ The loop condition is an arbitrary expression, which contains the
+ current loop iterator
Re: [GSoC] generation of Gimple loops with empty bodies
> I would use -x to see the headers. This should tell you if it is defined
> or only used there.
It gives the following output:
roman@roman-System-Product-Name:~/isl-0.12.2/lib$ objdump -x
libisl.so.10.2.2 | grep isl_ast_expr_get_val
00060380 g F .text 0053
isl_ast_expr_get_val
> Sorry, but this still needs to be debugged. :-( I am too busy to do it
> myself, so i am afraid you may just need to dig into it yourself. :-(
I've found the reason of this error. Somehow cc1 used an older version
of isl provided by the package from the standard repository in spite
of manually selected paths to isl 0.12.2. I think it is better to move
on and not to concentrate on this.
>> +#include "graphite-htab.h"
>
> Is this still needed?
No, this is redundant.
>> +/* IVS_PARAMS_MAP maps ISL's scattering and parameter identifiers
>> + to corresponding trees. */
>> +typedef struct ivs_params {
>> + std::map ivs_params_map;
>
> What about calling it isl_id_to_tree?
Maybe it would be better to call it tree_from_isl_id (because it would
help to avoid confusion with definitions from isl)?
>> + sese region;
>
> Is this region needed?
No, this is redundant.
>> + We create a new if region protecting the loop to be executed, if
>> + the execution count is zero (lower bound > upper bound). In this case,
>> + it is subsequently removed by dead code elimination. */
>
>
> The last sentence is unclear. Are you saying the loop will be removed or
> that the condition will be removed in case it is always true. Also, what
> about cases where the condition remains?
I wanted to say that the loop will be removed. Maybe it would be
better to remove the last sentence at all (because the previous
sentence already explains the motivation)?
> Those changes are unrelated and are part of the other patch, right?
Yes, these changes were made in the revision under No. 212361.
>> + ip.region = region;
>
> Is this needed? ip.region seems unused.
Yes, this is redundant.
I tried to incorporate all your comments in the attached patch.
--
Cheers, Roman Gareev.
Index: gcc/graphite-isl-ast-to-gimple.c
===
--- gcc/graphite-isl-ast-to-gimple.c(revision 212361)
+++ gcc/graphite-isl-ast-to-gimple.c(working copy)
@@ -25,7 +25,14 @@
#include
#include
#include
+#if defined(__cplusplus)
+extern "C" {
#endif
+#include
+#if defined(__cplusplus)
+}
+#endif
+#endif
#include "system.h"
#include "coretypes.h"
@@ -42,6 +49,9 @@
#include "cfgloop.h"
#include "tree-data-ref.h"
#include "sese.h"
+#include "tree-ssa-loop-manip.h"
+#include "tree-scalar-evolution.h"
+#include
#ifdef HAVE_cloog
#include "graphite-poly.h"
@@ -52,6 +62,517 @@
static bool graphite_regenerate_error;
+/* We always use signed 128, until isl is able to give information about
+types */
+
+static tree *graphite_expression_size_type = &int128_integer_type_node;
+
+/* Converts a GMP constant VAL to a tree and returns it. */
+
+static tree
+gmp_cst_to_tree (tree type, mpz_t val)
+{
+ tree t = type ? type : integer_type_node;
+ mpz_t tmp;
+
+ mpz_init (tmp);
+ mpz_set (tmp, val);
+ wide_int wi = wi::from_mpz (t, tmp, true);
+ mpz_clear (tmp);
+
+ return wide_int_to_tree (t, wi);
+}
+
+/* Verifies properties that GRAPHITE should maintain during translation. */
+
+static inline void
+graphite_verify (void)
+{
+#ifdef ENABLE_CHECKING
+ verify_loop_structure ();
+ verify_loop_closed_ssa (true);
+#endif
+}
+
+/* TREE_FROM_ISL_ID maps ISL's scattering and parameter identifiers
+ to corresponding trees. */
+
+typedef struct ivs_params {
+ std::map tree_from_isl_id;
+} *ivs_params_p;
+
+/* Free all memory allocated for ISL's identifiers. */
+
+void ivs_params_clear (ivs_params_p ip)
+{
+ std::map::iterator it;
+ for (it = ip->tree_from_isl_id.begin ();
+ it != ip->tree_from_isl_id.end (); it++)
+{
+ isl_id_free (it->first);
+}
+}
+
+static tree
+gcc_expression_from_isl_expression (tree type, __isl_take isl_ast_expr *,
+ ivs_params_p ip);
+
+/* Return the tree variable that corresponds to the given isl ast identifier
+ expression (an isl_ast_expr of type isl_ast_expr_id). */
+
+static tree
+gcc_expression_from_isl_ast_expr_id (__isl_keep isl_ast_expr *expr_id,
+ivs_params_p ip)
+{
+ gcc_assert (isl_ast_expr_get_type (expr_id) == isl_ast_expr_id);
+ isl_id *tmp_isl_id = isl_ast_expr_get_id (expr_id);
+ std::map::iterator res;
+ res = ip->tree_from_isl_id.find (tmp_isl_id);
+ isl_id_free (tmp_isl_id);
+ gcc_assert (res != ip->tree_from_isl_id.end () &&
+ "Could not map isl_id to tree expression");
+ isl_ast_expr_free (expr_id);
+ return res->second;
+}
+
+/* Converts an isl_ast_expr_int expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+gcc_expression_from_isl_expr_int (tree type, __isl_tak
Re: [GSoC] generation of Gimple loops with empty bodies
I tried to incorporate all your comments in the attached patch.
Nice. It looks now very good to me. One final and last detail:
+/* TREE_FROM_ISL_ID maps ISL's scattering and parameter identifiers
+ to corresponding trees. */
+
+typedef struct ivs_params {
+ std::map tree_from_isl_id;
+} *ivs_params_p;
The struct now contains only a single element such that there seems to
be no need for it anymore. Should we remove it? (We could still use a
typedef if you believe the datatype is too long).
Cheers,
Tobias
Re: [GSoC] generation of Gimple loops with empty bodies
> The struct now contains only a single element such that there seems to be no > need for it anymore. Should we remove it? (We could still use a typedef if > you believe the datatype is too long). I don't mind removing it. However I think that we should choose the way of transferring of sese region, which is used for translation of an isl_ast_node_user to Gimple code. Should we transfer it separately or restore ivs_params later? What do you think? -- Cheers, Roman Gareev.
Re: [GSoC] generation of Gimple loops with empty bodies
On 11/07/2014 13:11, Roman Gareev wrote: The struct now contains only a single element such that there seems to be no need for it anymore. Should we remove it? (We could still use a typedef if you believe the datatype is too long). I don't mind removing it. However I think that we should choose the way of transferring of sese region, which is used for translation of an isl_ast_node_user to Gimple code. Should we transfer it separately or restore ivs_params later? What do you think? Sorry, I currently don't see why sese region is needed later. In case it is, we may keep it, but this would require to pull more code in the review. I personally would just remove it to finish this review. Feel free to commit after applying this change. Cheers, Tobias
Re: [GSoC] generation of Gimple loops with empty bodies
I've attached an improved version of the patch and the ChangeLog
entry. Are they fine for trunk?
--
Cheers, Roman Gareev.
2014-07-11 Roman Gareev
gcc/
* graphite-isl-ast-to-gimple.c (gmp_cst_to_tree):
New function.
(graphite_verify): New function.
(ivs_params_clear): New function.
(gcc_expression_from_isl_ast_expr_id): New function.
(gcc_expression_from_isl_expr_int): New function.
(binary_op_to_tree): New function.
(ternary_op_to_tree): New function.
(unary_op_to_tree): New function.
(nary_op_to_tree): New function.
(gcc_expression_from_isl_expr_op): New function.
(gcc_expression_from_isl_expression): New function.
(graphite_create_new_loop): New function.
(translate_isl_ast_for_loop): New function.
(get_upper_bound): New function.
(graphite_create_new_loop_guard): New function.
(translate_isl_ast_node_for): New function.
(translate_isl_ast): New function.
(add_parameters_to_ivs_params): New function.
(scop_to_isl_ast): New parameter ip.
(graphite_regenerate_ast_isl): Add generation of GIMPLE code.
Index: gcc/graphite-isl-ast-to-gimple.c
===
--- gcc/graphite-isl-ast-to-gimple.c(revision 212450)
+++ gcc/graphite-isl-ast-to-gimple.c(working copy)
@@ -25,7 +25,14 @@
#include
#include
#include
+#if defined(__cplusplus)
+extern "C" {
#endif
+#include
+#if defined(__cplusplus)
+}
+#endif
+#endif
#include "system.h"
#include "coretypes.h"
@@ -42,6 +49,9 @@
#include "cfgloop.h"
#include "tree-data-ref.h"
#include "sese.h"
+#include "tree-ssa-loop-manip.h"
+#include "tree-scalar-evolution.h"
+#include
#ifdef HAVE_cloog
#include "graphite-poly.h"
@@ -52,6 +62,515 @@
static bool graphite_regenerate_error;
+/* We always use signed 128, until isl is able to give information about
+types */
+
+static tree *graphite_expression_size_type = &int128_integer_type_node;
+
+/* Converts a GMP constant VAL to a tree and returns it. */
+
+static tree
+gmp_cst_to_tree (tree type, mpz_t val)
+{
+ tree t = type ? type : integer_type_node;
+ mpz_t tmp;
+
+ mpz_init (tmp);
+ mpz_set (tmp, val);
+ wide_int wi = wi::from_mpz (t, tmp, true);
+ mpz_clear (tmp);
+
+ return wide_int_to_tree (t, wi);
+}
+
+/* Verifies properties that GRAPHITE should maintain during translation. */
+
+static inline void
+graphite_verify (void)
+{
+#ifdef ENABLE_CHECKING
+ verify_loop_structure ();
+ verify_loop_closed_ssa (true);
+#endif
+}
+
+/* IVS_PARAMS maps ISL's scattering and parameter identifiers
+ to corresponding trees. */
+
+typedef std::map ivs_params;
+
+/* Free all memory allocated for ISL's identifiers. */
+
+void ivs_params_clear (ivs_params &ip)
+{
+ std::map::iterator it;
+ for (it = ip.begin ();
+ it != ip.end (); it++)
+{
+ isl_id_free (it->first);
+}
+}
+
+static tree
+gcc_expression_from_isl_expression (tree type, __isl_take isl_ast_expr *,
+ ivs_params &ip);
+
+/* Return the tree variable that corresponds to the given isl ast identifier
+ expression (an isl_ast_expr of type isl_ast_expr_id). */
+
+static tree
+gcc_expression_from_isl_ast_expr_id (__isl_keep isl_ast_expr *expr_id,
+ivs_params &ip)
+{
+ gcc_assert (isl_ast_expr_get_type (expr_id) == isl_ast_expr_id);
+ isl_id *tmp_isl_id = isl_ast_expr_get_id (expr_id);
+ std::map::iterator res;
+ res = ip.find (tmp_isl_id);
+ isl_id_free (tmp_isl_id);
+ gcc_assert (res != ip.end () &&
+ "Could not map isl_id to tree expression");
+ isl_ast_expr_free (expr_id);
+ return res->second;
+}
+
+/* Converts an isl_ast_expr_int expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+gcc_expression_from_isl_expr_int (tree type, __isl_take isl_ast_expr *expr)
+{
+ gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_int);
+ isl_val *val = isl_ast_expr_get_val (expr);
+ mpz_t val_mpz_t;
+ mpz_init (val_mpz_t);
+ tree res;
+ if (isl_val_get_num_gmp (val, val_mpz_t) == -1)
+res = NULL_TREE;
+ else
+res = gmp_cst_to_tree (type, val_mpz_t);
+ isl_val_free (val);
+ isl_ast_expr_free (expr);
+ mpz_clear (val_mpz_t);
+ return res;
+}
+
+/* Converts a binary isl_ast_expr_op expression E to a GCC expression tree of
+ type TYPE. */
+
+static tree
+binary_op_to_tree (tree type, __isl_take isl_ast_expr *expr, ivs_params &ip)
+{
+ isl_ast_expr *arg_expr = isl_ast_expr_get_op_arg (expr, 0);
+ tree tree_lhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
+ arg_expr = isl_ast_expr_get_op_arg (expr, 1);
+ tree tree_rhs_expr = gcc_expression_from_isl_expression (type, arg_expr, ip);
+ enum isl_ast_op_type expr_type = isl_ast_expr_get_op_type (expr);
+ isl_ast_expr_free (expr);
+ switch (expr_type)
+{
+case
Re: [GSoC] generation of Gimple loops with empty bodies
On 11/07/2014 15:41, Roman Gareev wrote: I've attached an improved version of the patch and the ChangeLog entry. Are they fine for trunk? LGTM. Thank you! Tobias
Re: [GSoC] generation of Gimple loops with empty bodies
On x86_64-apple-darwin13, the test gcc.dg/graphite/isl-codegen-loop-dumping.c gives an ICE with -m32 after r212455. The backtrace is * thread #1: tid = 0xdac306, 0x000100523c52 cc1`fold_binary_loc(loc=0, code=MINUS_EXPR, type=0x, op0=0x000142c09bd0, op1=0x000142c1b048) + 6402 at fold-const.c:10813, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) frame #0: 0x000100523c52 cc1`fold_binary_loc(loc=0, code=MINUS_EXPR, type=0x, op0=0x000142c09bd0, op1=0x000142c1b048) + 6402 at fold-const.c:10813 10810 if (TREE_CODE (type) != COMPLEX_TYPE 10811 && TREE_CODE (arg0) == NEGATE_EXPR 10812 && integer_onep (arg1) -> 10813 && !TYPE_OVERFLOW_TRAPS (type)) 10814return fold_build1_loc (loc, BIT_NOT_EXPR, type, 10815fold_convert_loc (loc, type, 10816 TREE_OPERAND (arg0, 0))); (lldb) bt * thread #1: tid = 0xdac306, 0x000100523c52 cc1`fold_binary_loc(loc=0, code=MINUS_EXPR, type=0x, op0=0x000142c09bd0, op1=0x000142c1b048) + 6402 at fold-const.c:10813, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x000100523c52 cc1`fold_binary_loc(loc=0, code=MINUS_EXPR, type=0x, op0=0x000142c09bd0, op1=0x000142c1b048) + 6402 at fold-const.c:10813 frame #1: 0x000100549c2b cc1`fold_build2_stat_loc(loc=0, code=MINUS_EXPR, type=0x, op0=0x000142c09bd0, op1=0x000142c1b048) + 27 at fold-const.c:14985 frame #2: 0x0001005ff9a1 cc1`gcc_expression_from_isl_expression + 417 at graphite-isl-ast-to-gimple.c:175 frame #3: 0x0001005ff800 cc1`gcc_expression_from_isl_expression + 928 frame #4: 0x0001005ff818 cc1`gcc_expression_from_isl_expression + 24 at graphite-isl-ast-to-gimple.c:164 frame #5: 0x0001005ff800 cc1`gcc_expression_from_isl_expression + 928 frame #6: 0x0001005ffd26 cc1`translate_isl_ast + 229 at graphite-isl-ast-to-gimple.c:501 frame #7: 0x0001005ffc41 cc1`translate_isl_ast + 17 frame #8: 0x0001005ffc30 cc1`translate_isl_ast(context_loop=0x000142c031a0, node=0x000141f1acd0, next_e=0x000142d36f50, ip=0x7fff5fbfeff0) + 128 frame #9: 0x00010060047c cc1`graphite_regenerate_ast_isl(scop=) + 844 at graphite-isl-ast-to-gimple.c:699 frame #10: 0x0001005f8a28 cc1`graphite_transform_loops() + 2280 at graphite.c:304 frame #11: 0x0001005f8ab1 cc1`execute(this=, fun=) + 33 at graphite.c:333 frame #12: 0x000100783097 cc1`execute_one_pass(pass=0x000141e10f40) + 407 at passes.c:2149 frame #13: 0x00010078368e cc1`execute_pass_list_1(pass=0x000141e10f40) + 30 at passes.c:2201 frame #14: 0x0001007836a0 cc1`execute_pass_list_1(pass=0x000141e10ee0) + 48 at passes.c:2202 frame #15: 0x0001007836a0 cc1`execute_pass_list_1(pass=0x000141e10ac0) + 48 at passes.c:2202 frame #16: 0x0001007836a0 cc1`execute_pass_list_1(pass=0x000141e0fa40) + 48 at passes.c:2202 frame #17: 0x0001007836e9 cc1`execute_pass_list(fn=0x000142d1c738, pass=0x000141e0f980) + 25 at passes.c:2212 frame #18: 0x0001003ee215 cc1`expand_function(node=0x000142d3b000) + 245 at cgraphunit.c:1786 frame #19: 0x0001003f0847 cc1`compile() + 230 at cgraphunit.c:1920 frame #20: 0x0001003f0761 cc1`compile() + 2209 frame #21: 0x0001003f1026 cc1`finalize_compilation_unit() + 102 at cgraphunit.c:2341 frame #22: 0x000100020d55 cc1`c_write_global_declarations() + 613 at c-decl.c:10463 frame #23: 0x00010085cc97 cc1`compile_file + 167 at toplev.c:562 frame #24: 0x00010085f43f cc1`toplev_main(argc=7, argv=0x7fff5fbff390) + 3327 at toplev.c:1946 TIA Dominique
Re: [GSoC] generation of Gimple loops with empty bodies
Hi Dominique, thank you for the message! I've attached a patch, that may fix the issue. Please report back if it fixes the problem. -- Cheers, Roman Gareev. Index: gcc/graphite-isl-ast-to-gimple.c === --- gcc/graphite-isl-ast-to-gimple.c(revision 212491) +++ gcc/graphite-isl-ast-to-gimple.c(working copy) @@ -65,7 +65,9 @@ /* We always use signed 128, until isl is able to give information about types */ -static tree *graphite_expression_size_type = &int128_integer_type_node; +static tree *graphite_expression_size_type = int128_integer_type_node ? +&int128_integer_type_node : +&long_long_integer_type_node; /* Converts a GMP constant VAL to a tree and returns it. */
Re: [GSoC] generation of Gimple loops with empty bodies
Hi Roman, Thanks for the quick answer. > Please report back if it fixes the problem. I have not yet done a full regtest, but a manual testing suggest that the patch fixes the problem. Dominique
Re: [GSoC] generation of Gimple loops with empty bodies
I have regtested graphite.exp for gcc/g++/gfortran/libgomp without regression. So your patch seems a safe fix. Dominique
Re: [GSoC] generation of Gimple loops with empty bodies
On 13/07/2014 12:34, Roman Gareev wrote: Hi Dominique, thank you for the message! I've attached a patch, that may fix the issue. Please report back if it fixes the problem. Roman, this patch is OK to commit, but please include a correct changelog file. Tobias
Re: [GSoC] generation of Gimple loops with empty bodies
I've found out that int128_integer_type_node and long_long_integer_type_node are NULL at the moment of definition of the graphite_expression_size_type. Maybe we should use long_long_integer_type_node, because, as you said before, using of signed 64 has also been proved to be very robust. What do you think about this? -- Cheers, Roman Gareev.
Re: [GSoC] generation of Gimple loops with empty bodies
> I suggest you use the largest available integer mode via > mode = mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0); > type = build_nonstandard_integer_type (GET_MODE_PRECISION (mode), [01]); Thank you for the suggestion! > Roman, can you give this a shot? Maybe, we could use the following code: static int max_mode_int_precision = GET_MODE_PRECISION (mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0)); static int graphite_expression_type_precision = 128 <= max_mode_int_precision ? 128 : max_mode_int_precision; This allows us to change the size during debugging and avoid using unacceptable mode size. Tobias, what do you think about this? -- Cheers, Roman Gareev 2014-07-08 Roman Gareev gcc/ * graphite-isl-ast-to-gimple.c: Add using of build_nonstandard_integer_type instead of int128_integer_type_node. Index: gcc/graphite-isl-ast-to-gimple.c === --- gcc/graphite-isl-ast-to-gimple.c(revision 212804) +++ gcc/graphite-isl-ast-to-gimple.c(working copy) @@ -62,10 +62,13 @@ static bool graphite_regenerate_error; -/* We always use signed 128, until isl is able to give information about -types */ +/* We always use signed 128, until it is not accpeted or isl is able to give + information about types. */ -static tree *graphite_expression_size_type = &int128_integer_type_node; +static int max_mode_int_precision = + GET_MODE_PRECISION (mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0)); +static int graphite_expression_type_precision = 128 <= max_mode_int_precision ? + 128 : max_mode_int_precision; /* Converts a GMP constant VAL to a tree and returns it. */ @@ -494,7 +497,8 @@ tree cond_expr; edge exit_edge; - *type = *graphite_expression_size_type; + *type = +build_nonstandard_integer_type (graphite_expression_type_precision, 0); isl_ast_expr *for_init = isl_ast_node_for_get_init (node_for); *lb = gcc_expression_from_isl_expression (*type, for_init, ip); isl_ast_expr *upper_bound = get_upper_bound (node_for);
Re: [GSoC] generation of Gimple loops with empty bodies
On 18/07/2014 12:34, Roman Gareev wrote: I suggest you use the largest available integer mode via >mode = mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0); >type = build_nonstandard_integer_type (GET_MODE_PRECISION (mode), [01]); Thank you for the suggestion! >Roman, can you give this a shot? Maybe, we could use the following code: static int max_mode_int_precision = GET_MODE_PRECISION (mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0)); static int graphite_expression_type_precision = 128 <= max_mode_int_precision ? 128 : max_mode_int_precision; This allows us to change the size during debugging and avoid using unacceptable mode size. Tobias, what do you think about this? One comment is unclear. Otherwise it is good. If my proposed comment is OK, please update, commit and close the bug report. Tobias -- Cheers, Roman Gareev ChangeLog_entry.txt 2014-07-08 Roman Gareev gcc/ * graphite-isl-ast-to-gimple.c: Add using of build_nonstandard_integer_type instead of int128_integer_type_node. patch.txt Index: gcc/graphite-isl-ast-to-gimple.c === --- gcc/graphite-isl-ast-to-gimple.c(revision 212804) +++ gcc/graphite-isl-ast-to-gimple.c(working copy) @@ -62,10 +62,13 @@ static bool graphite_regenerate_error; -/* We always use signed 128, until isl is able to give information about -types */ +/* We always use signed 128, until it is not accpeted or isl is able to give + information about types. */ This is not understandable. What about; /* We always try to use signed 128 bit types, but fall back to smaller types in case a platform does not provide types of this sizes. In the future we should use isl to derive the optimal type for each subexpression. */ -static tree *graphite_expression_size_type = &int128_integer_type_node; +static int max_mode_int_precision = + GET_MODE_PRECISION (mode_for_size (MAX_FIXED_MODE_SIZE, MODE_INT, 0)); +static int graphite_expression_type_precision = 128 <= max_mode_int_precision ? + 128 : max_mode_int_precision; /* Converts a GMP constant VAL to a tree and returns it. */ @@ -494,7 +497,8 @@ tree cond_expr; edge exit_edge; - *type = *graphite_expression_size_type; + *type = +build_nonstandard_integer_type (graphite_expression_type_precision, 0); isl_ast_expr *for_init = isl_ast_node_for_get_init (node_for); *lb = gcc_expression_from_isl_expression (*type, for_init, ip); isl_ast_expr *upper_bound = get_upper_bound (node_for);
