PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

The array same_range method has issues

Open sergisiso opened this issue 1 year ago • 4 comments

The same_range method, that I am moving to ArrayMixin in #2465, has false negative: cases when the range is the same but the method returns false. This is currently fine because we just act in some transformation when we find a positive, but it could be improved to perform more valid transformations.

sergisiso avatar Jan 19 '24 14:01 sergisiso

I am also finding false positive, which are more concerning because they lead to wrong arrayrange2loop transformations. For example for a(:) = b(:) we say its the same range because its valid Fortran, which imply that their length matches. However, we need to prove that the lower bound on the declaration is the same, as if they are not, the given expression will shift the indexes.

sergisiso avatar May 28 '24 14:05 sergisiso

I am also finding false positive, which are more concerning because they lead to wrong arrayrange2loop transformations. For example for a(:) = b(:) we say its the same range because its valid Fortran, which imply that their length matches. However, we need to prove that the lower bound on the declaration is the same, as if they are not, the given expression will shift the indexes.

Now that I read this I realise I fixed a very similar issue to this in our handling of WHERE blocks. We don't have to find the declaration because, if we can't, we can use the Fortran LBOUND intrinsic.

arporter avatar May 28 '24 15:05 arporter

There may already be a utility method to do this in fact.

arporter avatar May 28 '24 15:05 arporter

Aha, good point! So I don't need the associate/mold that I was talking this afternoon! If I understand correctly, the same_range is still wrong for these cases because it should return False conservatively.

Then if it's returns true, we can do (as we do now):

do i= LBOUND(a, 1), UBOUND(a, 1)
   a(i) = b(i)
end do

but if it returns false (regardless of if it's a false negative or not) we should do:

offset =  LBOUND(b, 1) - LBOUND(a, 1)
do i= LBOUND(a, 1), UBOUND(a, 1)
   a(i) = b(i + offset)
end do

(... ofc with the offset expression in-place as we don't want to move data arround and lbounds expressions are probably something the compiler knows and does constant folding)

sergisiso avatar May 28 '24 17:05 sergisiso