CloogOptions->esp usage in CLooG-Parma
Sebastian Pop
Even though we are setting
options->esp = 1;
with CLooG-Parma, I am still seeing a clast_assignment generated
for the attached cloog input file:
[...]
scat_1 = 1;
for (scat_3=51*scat_1;scat_3<=63;scat_3++) {
[...]
CLooG-ISL does not seem to produce clast_assignments with
the esp option. Is this a problem in CLooG-Parma?
Thanks,
Sebastian
Sven Verdoolaege
> Hi Sven,
>
> Even though we are setting
> options->esp = 1;
You don't need to set this explicitly. It's the default.
> with CLooG-Parma, I am still seeing a clast_assignment generated
> for the attached cloog input file:
>
> [...]
> scat_1 = 1;
> for (scat_3=51*scat_1;scat_3<=63;scat_3++) {
> [...]
Does that bother you?
> CLooG-ISL does not seem to produce clast_assignments with
> the esp option. Is this a problem in CLooG-Parma?
isl is just better at detecting equalities.
The patch below will get rid of the assignment above,
but I'm not sure if it is worth it.
In any case, CLooG will not propagate equalities involving
floors, even with the isl backend. See, e.g., test/basic-bounds-5.c
j = floord(M+1,2) ;
S1(1,j) ;
Removing these would be a lot more work.
skimo
diff --git a/source/clast.c b/source/clast.c
index 33e0a7b..f19f525 100644
--- a/source/clast.c
+++ b/source/clast.c
@@ -52,7 +52,8 @@ static void insert_guard(CloogConstraintSet *constraints, int level,
static int insert_modulo_guard(CloogConstraint *upper,
CloogConstraint *lower, int level,
struct clast_stmt ***next, CloogInfos *infos);
-static int insert_equation(CloogDomain *domain, CloogConstraint *upper,
+static int insert_equation(CloogDomain *domain, CloogConstraintSet *constraints,
+ CloogConstraint *upper,
CloogConstraint *lower, int level,
struct clast_stmt ***next, CloogInfos *infos);
static int insert_for(CloogDomain *domain, CloogConstraintSet *constraints,
@@ -1474,12 +1475,15 @@ static int insert_equation_as_loop(CloogDomain *domain, CloogConstraint *upper,
* - July 14th 2003: (debug) no more print the constant in the modulo guard when
* it was previously included in a stride calculation.
*/
-static int insert_equation(CloogDomain *domain, CloogConstraint *upper,
+static int insert_equation(CloogDomain *domain, CloogConstraintSet *constraints,
+ CloogConstraint *upper,
CloogConstraint *lower, int level, struct clast_stmt
***next, CloogInfos *infos)
{
struct clast_expr *e;
struct clast_assignment *ass;
+ CloogConstraint *constraint;
+ int equality = 1;
if (!infos->options->otl)
return insert_equation_as_loop(domain, upper, lower, level, next, infos);
@@ -1491,8 +1495,20 @@ static int insert_equation(CloogDomain *domain, CloogConstraint *upper,
return 0;
}
- if (cloog_constraint_is_valid(lower) ||
- !clast_equal_add(infos->equal, NULL, level, upper, infos))
+ if (cloog_constraint_is_valid(lower)) {
+ struct clast_expr *e1;
+ struct clast_expr *e2;
+ e1 = clast_bound_from_constraint(lower, level, infos->names);
+ e2 = clast_bound_from_constraint(upper, level, infos->names);
+ if (!clast_expr_equal(e1, e2))
+ equality = 0;
+ free_clast_expr(e1);
+ free_clast_expr(e2);
+ }
+
+ constraint = cloog_constraint_is_valid(lower) ? NULL : upper;
+ if (!equality ||
+ !clast_equal_add(infos->equal, constraints, level, constraint, infos))
{ /* Finally, the equality. */
/* If we have to make a block by dimension, we start the block. Function
@@ -1767,15 +1783,15 @@ static void insert_loop(CloogLoop * loop, int level,
*/
if (cloog_constraint_is_valid(i =
cloog_constraint_set_defining_equality(constraints, level))) {
- empty_loop = !insert_equation(loop->unsimplified, i,
+ empty_loop = !insert_equation(loop->unsimplified, constraints,i,
cloog_constraint_invalid(), level, next,
infos);
equality = 1 ;
} else if (cloog_constraint_is_valid(i =
cloog_constraint_set_defining_inequalities(constraints,
level, &j, infos->names->nb_parameters))) {
- empty_loop = !insert_equation(loop->unsimplified, i, j, level, next,
- infos);
+ empty_loop = !insert_equation(loop->unsimplified, constraints, i, j,
+ level, next, infos);
} else
empty_loop = !insert_for(loop->unsimplified, constraints, level,
loop->otl, next, infos);
Sebastian Pop
>> with CLooG-Parma, I am still seeing a clast_assignment generated
>> for the attached cloog input file:
>>
>> [...]
>> scat_1 = 1;
>> for (scat_3=51*scat_1;scat_3<=63;scat_3++) {
>> [...]
>
> Does that bother you?
A little bit: as Graphite doesn't handle assignments, we end up with an ICE, ...
>> CLooG-ISL does not seem to produce clast_assignments with
>> the esp option. Is this a problem in CLooG-Parma?
>
> isl is just better at detecting equalities.
> The patch below will get rid of the assignment above,
> but I'm not sure if it is worth it.
>
> In any case, CLooG will not propagate equalities involving
> floors, even with the isl backend. See, e.g., test/basic-bounds-5.c
>
> j = floord(M+1,2) ;
> S1(1,j) ;
>
> Removing these would be a lot more work.
>
... and so this tells me that independently of the improvement of CLooG-Parma,
I will have to teach Graphite how to translate assignments.
Thanks for the heads up.
Sebastian
Sven Verdoolaege
Yeah... I don't think I'll ever get rid of those in CLooG-Parma.
Maybe if one day we get rid of the other backends, I'll do it
in the isl one.
So, do you want me to apply the patch?
skimo
Sebastian Pop
> So, do you want me to apply the patch?
It would be an improvement, no? So please commit the patch.
Thanks,
Sebastian
Sven Verdoolaege
> On Mon, Mar 14, 2011 at 05:37, Sven Verdoolaege <skimo...@kotnet.org> wrote:
> > So, do you want me to apply the patch?
>
> It would be an improvement, no?
Only for the non-isl backends. For isl, it would just be extra
code to execute. It's also more code that I have to maintain
(and remember to throw out again if we ever throu out the other
backends).
skimo
Sebastian Pop
> Only for the non-isl backends. For isl, it would just be extra
> code to execute. It's also more code that I have to maintain
> (and remember to throw out again if we ever throu out the other
> backends).
These are valid reasons to not commit the patch.
Sebastian
Sven Verdoolaege
OK, so given that there is a cost involved, do you still want
me to apply the patch?
That is, are you actually going to use cloog-parma?
skimo
Sebastian Pop
> OK, so given that there is a cost involved, do you still want
> me to apply the patch?
No, I do not want you to commit a patch that would slow down the ISL backend.
Sorry for not being clear in my previous email.
> That is, are you actually going to use cloog-parma?
Yes, GCC is able to configure and use the cloog-parma backend.
Sebastian
Prof. Roberto Bagnara
:-D
I am amazed by the fact that almost all messages by Sven on this
list contain some (more or less direct) anti-PPL statement.
Cheers,
Roberto
--
Prof. Roberto Bagnara
Applied Formal Methods Laboratory
Department of Mathematics, University of Parma, Italy
http://www.cs.unipr.it/~bagnara/
mailto:bag...@cs.unipr.it
Sven Verdoolaege
> On Mon, Mar 14, 2011 at 12:40, Sven Verdoolaege <skimo...@kotnet.org> wrote:
> > OK, so given that there is a cost involved, do you still want
> > me to apply the patch?
>
> No, I do not want you to commit a patch that would slow down the ISL backend.
> Sorry for not being clear in my previous email.
OK. Note that I have no idea how much of a slow-down you would get.
It may be completely insignificant.
> > That is, are you actually going to use cloog-parma?
>
> Yes, GCC is able to configure and use the cloog-parma backend.
I know that. I justed want to know if you were planning
on making this a recommended configuration. If you are,
then it may still be worth it to apply the patch.
On the other hand, if you are going to handle assignments
in graphite anyway, then it may not matter all that much.
skimo