[GSoC] type of an isl_ast_expr_id

[Tobias Grosser]

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