On 29/07/2014 12:14, Roman Gareev wrote:
I've tested Graphite with the ISL AST generator as the main code
generator and found out the following problem: in the current
implementation the gcc_expression_from_isl_ast_expr_id can return a
tree of a type, which doesn't correspond to the type chosen for
graphite expressions.
[..]
It is caused by difference in precisions of arguments in the min
expression. The attached patch may fix this.
Great.
Blindly converting type sizes is not a good idea and may be problematic
when we switch to smaller types. As we can obviously only improve this
when we have the appropriate support in isl, I think the attached patch
is fine. However, please add a fixme that states that this should
be looked at again at the moment when we get isl support to derive the
optimal types.
I've also found out that pr35356-2.c is no longer suitable for
testing. The following CLooG AST
for (scat_1=0;scat_1<=min(k_6-1,n_5-1);scat_1++) {
(scat_1);
}
if ((k_6 >= 0) && (n_5 >= k_6+1)) {
(k_6);
}
for (scat_1=max(0,k_6+1);scat_1<=n_5-1;scat_1++) {
(scat_1);
}
is generated from its source code
int a[100];
int
foo (int bar, int n, int k)
{
int i;
for (i = 0; i < n; i++)
if (i == k)
a[i] = 1;
else
a[i] = i;
return a[bar];
}
The test checks that the dump file contains four MIN_EXPR and four MAX_EXPR
/* { dg-final { scan-tree-dump-times "MIN_EXPR\[^\\n\\r]*;" 4 "graphite" } } */
/* { dg-final { scan-tree-dump-times "MAX_EXPR\[^\\n\\r]*;" 4 "graphite" } } */
However, the ISL AST generated from this code
if (k >= 1) {
for (int c1 = 0; c1 < n; c1 += 1)
if (c1 >= k + 1) {
S_7(c1);
} else if (k >= c1 + 1) {
S_7(c1);
} else
S_6(k);
} else {
if (k == 0)
S_6(0);
for (int c1 = max(k + 1, 0); c1 < n; c1 += 1)
S_7(c1);
}
doesn't contain min expression at all.
Should we remove /* { dg-final { scan-tree-dump-times
"MIN_EXPR\[^\\n\\r]*;" 4 "graphite" } } */ from this test case?
Please have a look at the original bug report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=35356
The isl ast generator should produce something like:
for (i = 0; i < min (n, k); i++)
a[i] = i;
if (k >= 0 && k < n)
a[k] = 1;
for (i = max(k+1,0); i < n; i++)
a[i] = i;
CLooG does not generate optimal code either, but the code generated by
isl is a regression compared to CLooG.
Can you generate an isl_codegen input for this test case and verify that
the latest public version of isl does not generate this optimal code
either. If it does not, please report this as an optimization bug to the
isl mailing list.
After you got feedback, I think it is OK to disable this test and to
add a FIXME explaining why this test is disabled and if we can expect a
fix in later versions of isl.
P.S.: The other graphite tests (except vect-pr43423.c) are passed by
graphite in case we run it with the ISL AST generator.
Amazing.
What is the problem with this last test case?
Cheers,
Tobias
