Reduced testcase for aermod in polyhedron benchmark.

[Li Feng]

Hi,

I have reduced the aermod problem in polyhedron benchmark.
It seems that the cloog generate the wrong code for a simple
case...

-----------Reduced fortran code--------------------
MODULE MAIN1
REAL , ALLOCATABLE :: HRVALD(:)
END MODULE MAIN1

SUBROUTINE VOLCALC()
USE MAIN1
INTEGER :: ITYP
LOGICAL :: WETSCIM

DO ITYP = 1 , 100
IF ( WETSCIM ) HRVALD(ITYP) = 0.0
ENDDO
END SUBROUTINE VOLCALC
---------------------------------------------------------
Equivalent C code
int hrval[101];

void volcalc()
{
int i;
int wetscim;

for (i = 0; i < 100; i++)
if (wetscim)
hrval[i] = 0;
}
--------------------------------------------------
(gdb) set debug_generated_program (scop)

if (wetscim_3 <= -1) {
for (scat_1=0;scat_1<=99;scat_1++) {
S4(scat_1) ;
}
}
if (wetscim_3 >= 1) {
for (;;scat_1++) {
for (git_0=0;git_0<=99;git_0++) {
S4(git_0) ;
}
}
}
----------------------------------------------------
(gdb) set debug_scop (scop)
scop (
parameters (
p_0 -> wetscim_3(D)
)
context (
# eq p0 cst
0 3
)
pbb_4 (
conditions bb_4 (if (wetscim_3(D) != 0)
)
cases bb_4 (if (wetscim_3(D) != 0)
)
pdr_0 (write
(Data Ref:
stmt: hrval[i_11] = 0;
ref: hrval[i_11];
base_object: hrval[0];
Access function 0: {0, +, 1}_1
)
data accesses (
# eq i0 p0 alias sub0 cst
4 6
0 1 0 0 -1 0
0 0 0 1 0 -1
1 -1 0 0 0 100
1 1 0 0 0 0
)
)
domains bb_4 (
# eq i0 p0 cst
3 4
1 -1 0 99
1 1 0 0
1 0 -1 -1
3 4
1 -1 0 99
1 1 0 0
1 0 1 -1
)
scattering bb_4 (
# eq s0 s1 s2 i0 p0 cst
3 7
0 1 0 0 0 0 0
0 0 1 0 -1 0 0
0 0 0 1 0 0 0
)
)
original_lst (
(root
0 (loop
0 stmt_4)))
transformed_lst (
(root
0 (loop
0 stmt_4)))
)

Li
On Fri, Oct 30, 2009 at 12:58 PM, Sebastian Pop <seb...@gmail.com> wrote:
> Hi Li,
>
> On Thu, Oct 29, 2009 at 04:54, Li Feng <nemok...@gmail.com> wrote:
>> Hi Sebastian,
>>
>> I have looked into the test results and see there are
>> 4 test suites fails in the benchmark.
>> 447.dealII & 454.calculix & 431.wrf miscompile with flag -O3
>> -fno-loop-strip-mine.
>> 447.dealII & 454.calculix & 416.gamess miscompile with flag -O2.
>>
>
> This is correct.
>
>> I may have time to try one of these bugs, but I've no idea which
>> has been already working on. So can you comment on these testcases
>> about their status?
>
> I am working on a patch to solve the calculix problem.  Konrad and
> Ramakrishna look at dealII.
>
>> It'll be great if you could attach these miscompiled unreduced files.
>
> Unfortunately I cannot send out the source of the SPEC benchmarks.
>
> But there is another fail in the polyhedron benchmark:
> http://www.polyhedron.com/web_images/documents/pb05.zip
>
> the testers report this:
>
> aermod FAILED    1 fails and    5 passes
> [...]
> Finished Testing  16 benchmarks -  15 passed, and   1 failed
>
> Could you look at aermod and try to reduce and understand the problem?
> You should open a bug report in gcc's bugzilla about this problem, and
> be sure to include "graphite miscompiles aermod" in the subject line.
>
> Thanks,
> Sebastian
>

[Albert Cohen]

Very strange indeed. Cedric is in CC.

I did not find any bug in the CLooG input.

Albert

[Cédric Bastoul]

Hi,
I wrote the .cloog file according to what I saw in your dump and I
don't see any problem with current or older CLooG versions... Maybe
the problem is somewhere else (are you sure this is the actual input
for CLooG (*) ?). Can you tell me more about the git_0 dimension, is
it one of the scattering dimensions (which one) ?

[CLooG] INFO: 2 dimensions (over 3) are scalar.
/* Generated from ./feng.cloog by CLooG 0.14.0-215-g772d615 gmp bits
in 0.00s. */
if (p0 >= 1) {
for (s1=0;s1<=99;s1++) {
S1(s1);
}
}
if (p0 <= -1) {
for (s1=0;s1<=99;s1++) {
S1(s1);
}
}

Cedric

(*) add "cloog_program_dump_cloog(stdout, program);" at the beginning
of cloog_program_generate(), set the "noscalars" option (very
important for this dump), and send me the output, it will help.

[Alexander Monakov]

Not sure if this matters but this (from Li's message):

(gdb) set debug_generated_program (scop)

if (wetscim_3 <= -1) {
for (scat_1=0;scat_1<=99;scat_1++) {
S4(scat_1) ;
}
}
if (wetscim_3 >= 1) {
for (;;scat_1++) {
for (git_0=0;git_0<=99;git_0++) {
S4(git_0) ;
}
}
}

looks highly suspicious: 'for (;;scat_1++)' denotes an infinite loop.
Li, do you confirm that aermod and the reduced testcase loop forever?

[Cédric Bastoul]

Yes, this happens only if there is no constraint for the scat_1
dimension. If the input for CLooG is not alterated in some way, I
suspect a bug in the cloog_loop_scatter() implementation for the PPL
backend (the official CLooG does not suffer from this).

Ced.

[Li Feng]

Hi Cédric,

I think I may have found the problem.

In Graphite code, where build cloog prog from the SCoP,
have the following lines:

for (i = 0; i < nbs; i++)
scaldims[i] = 0 ;

cloog_program_set_scaldims (prog, scaldims);

/* Extract scalar dimensions to simplify the code generation problem. */
cloog_program_extract_scalars (prog, scattering);

So it does nothing even if you set CloogOptions -noscalar to 1.
This might be the problem (as you have mentioned, this is important
for this problem, can you explain this?)

Before calling these lines:
The original scattering matrix is:

(gdb) set debug_cloog_domain (scattering->_domain)
3 7

eq s0 s1 s2 i0 p0 cst

0 1 0 0 0 0 0
0 0 1 0 -1 0 0
0 0 0 1 0 0 0

after cloog_program_extract_scalars call:
The scattering becomes:
1 5
eq s0 i0 p0 cst
0 1 -1 0 0
----------------------------------------------------------------------
The domains is broken after apply this scattering
by calling cloog_program_scatter:

(gdb) set debug_cloog_domain (scattering->_domain)

# Context (1 parameter(s)):
+-- CloogDomain
| [ 1 0 1 ]
|
1 # Parameter name(s)
wetscim_3

# Statement number:
1

# Iteration domain of statement 1.
2
5 5
0 1 -1 0 0
1 0 1 0 0
1 0 -1 0 99
1 0 0 -1 -1
1 0 0 0 1
4 5
1 0 1 0 0
1 0 -1 0 99
1 0 0 1 -1
1 0 0 0 1
0 0 0 # For future options.

1 # Iterator name(s)
scat_0 scat_1 scat_2 git_0

# No scattering functions.
0
-----------------------------------------------------------------
So we got broken after call cloog_program_generate:

(gdb) set debug_cloog_domain (scattering->_domain)

# Context (1 parameter(s)):
+-- CloogDomain
| [ 1 0 1 ]
|
1 # Parameter name(s)
wetscim_3

# Statement number:
2

# Iteration domain of statement 1.
1
4 4
1 1 0 0
1 -1 0 99
1 0 -1 -1
1 0 0 1
0 0 0 # For future options.

# Iteration domain of statement 2.
1
2 4
1 0 1 -1
1 0 0 1
0 0 0 # For future options.

1 # Iterator name(s)
scat_1 git_0

# No scattering functions.
0

And finally, this generate the strange code.

Cedic, what is your suggestion about this, we simply remove
call to cloog_program_extract_scalars and set noscalar?

Thanks,
Li

-----------------------------------------------------
ps. the orignal domain looks like:

# Context (1 parameter(s)):
+-- CloogDomain
| [ 1 0 1 ]
|
1 # Parameter name(s)
wetscim_3

# Statement number:
1

# Iteration domain of statement 1.
2

3 4
1 -1 0 99
1 1 0 0
1 0 -1 -1
3 4
1 -1 0 99
1 1 0 0
1 0 1 -1

0 0 0 # For future options.

1 # Iterator name(s)
scat_0 scat_1 scat_2 git_0

# No scattering functions.
0

[Li Feng]

Hi Alexander,

This strange loop will break compiling the code and cause a
segfault. So aermod will not run but miscompile by Graphite.

LI

[Cédric Bastoul]

Hi Li,
the scalar extraction seems correct, it's just an improvement to avoid
costly polyhedral operations for "trivial dimensions". The error is
where I suspected:

After the call to cloog_program_scatter you get:

> 2
> 5 5
> 0 1 -1 0 0
> 1 0 1 0 0
> 1 0 -1 0 99
> 1 0 0 -1 -1
> 1 0 0 0 1
> 4 5
> 1 0 1 0 0
> 1 0 -1 0 99
> 1 0 0 1 -1
> 1 0 0 0 1
> 0 0 0 # For future options.

While you should have:

> 2
> 5 5
> 0 1 -1 0 0
> 1 0 1 0 0
> 1 0 -1 0 99
> 1 0 0 -1 -1
> 1 0 0 0 1

> 5 5 # Updated number of rows
> 0 1 -1 0 0 # This row is the only change (same as in the first part)

> 1 0 1 0 0
> 1 0 -1 0 99
> 1 0 0 1 -1
> 1 0 0 0 1
> 0 0 0 # For future options.

The scattering is applied only to the first element of the union of
domains (== list of domains) instead to all the domain parts. Just a
missing loop somewhere, it should be very easy to correct. I can have
a look myself (that's in cloog-ppl, right ?), but not before this
week-end...

Ced.

[Tobias Grosser]

On Wed, 2009-11-04 at 13:45 -0500, Cédric Bastoul wrote:
> Hi Li,
> the scalar extraction seems correct, it's just an improvement to avoid
> costly polyhedral operations for "trivial dimensions". The error is
> where I suspected:
>
> After the call to cloog_program_scatter you get:

> > 2
> > 5 5
> > 0 1 -1 0 0
> > 1 0 1 0 0
> > 1 0 -1 0 99
> > 1 0 0 -1 -1
> > 1 0 0 0 1
> > 4 5
> > 1 0 1 0 0
> > 1 0 -1 0 99
> > 1 0 0 1 -1
> > 1 0 0 0 1
> > 0 0 0 # For future options.
>

> While you should have:

>
> > 2
> > 5 5
> > 0 1 -1 0 0
> > 1 0 1 0 0
> > 1 0 -1 0 99
> > 1 0 0 -1 -1
> > 1 0 0 0 1

> > 5 5 # Updated number of rows
> > 0 1 -1 0 0 # This row is the only change (same as in the first part)

> > 1 0 1 0 0
> > 1 0 -1 0 99
> > 1 0 0 1 -1
> > 1 0 0 0 1
> > 0 0 0 # For future options.
>

> The scattering is applied only to the first element of the union of
> domains (== list of domains) instead to all the domain parts. Just a
> missing loop somewhere, it should be very easy to correct. I can have
> a look myself (that's in cloog-ppl, right ?), but not before this
> week-end...

Hey Li,

just to be sure, what version of CLooG ppl are you using. I remember I
fixed one of these bugs in CLooG ppl, so you should have the current one

Tobi

[Li Feng]

Hi Tobi,

Sorry, I haven't update the cloog ppl for some time, so mine might be
not current. I'll download the new version and check again.

Li

[Li Feng]

Hi,

I updated my cloog-ppl and this bug became fixed. The aermod
in polyhedral benchmark passed the tests.
So this bug is resulted by the not up-to-date cloog version.

As Sebastian has suggested, to prevent from the similar error-prone action
happen again by the user, we should check the cloog version to make sure
it results in the correct code. I think this is necessary.
As the patch in this mail achieve:
http://gcc.gnu.org/ml/gcc-patches/2009-07/msg01839.html

The question is: which version of cloog is now good enough for Graphite?
>0.15.4 ?
I'd suggest that we commit this patch to trunk.

Li