This is an archive of the discontinued LLVM Phabricator instance.

[flang] Changed lowering for allocatable assignment to make array-value-copy correct.
ClosedPublic

Authored by vzakhari on Jul 7 2022, 11:40 AM.

Details

Summary

Array-value-copy fails to generate a temporary array for case like this:

subroutine bug(b)
  real, allocatable :: b(:)
  b = b(2:1:-1)
end subroutine

Since LHS may need to be reallocated, lowering produces the following FIR:

%rhs_load = fir.array_load %b %slice

%lhs_mem = fir.if %b_is_allocated_with_right_shape {
   fir.result %b
} else {
   %new_storage = fir.allocmem %rhs_shape
   fir.result %new_storage
}

%lhs = fir.array_load %lhs_mem
%loop = fir.do_loop {
 ....
}
fir.array_merge_store %lhs, %loop to %lhs_mem
// deallocate old storage if reallocation occured,
// and update b descriptor if needed.

Since %b in array_load and %lhs_mem in array_merge_store are not the same SSA
values, array-value-copy does not detect the conflict and does not produce
a temporary array. This causes incorrect result in runtime.

The suggested change in lowering is to generate this:

%rhs_load = fir.array_load %b %slice
%lhs_mem = fir.if %b_is_allocated_with_right_shape {
   %lhs = fir.array_load %b
   %loop = fir.do_loop {
      ....
   }
   fir.array_merge_store %lhs, %loop to %b
   fir.result %b
} else {
   %new_storage = fir.allocmem %rhs_shape
   %lhs = fir.array_load %new_storage
   %loop = fir.do_loop {
      ....
   }
   fir.array_merge_store %lhs, %loop to %new_storage
   fir.result %new_storage
}
// deallocate old storage if reallocation occured,
// and update b descriptor if needed.

Note that there are actually 3 branches in FIR, so the assignment loops
are currently produced in three copies, which is a code-size issue.
It is possible to generate just two branches with two copies of the loops,
but it is not addressed in this change-set.

Diff Detail

Event Timeline

vzakhari created this revision.Jul 7 2022, 11:40 AM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJul 7 2022, 11:40 AM
Herald added a subscriber: mehdi_amini. · View Herald Transcript
vzakhari requested review of this revision.Jul 7 2022, 11:40 AM
jeanPerier accepted this revision.Jul 8 2022, 2:44 AM

Thanks a lot for identifying and fixing this issue. LGTM.

This revision is now accepted and ready to land.Jul 8 2022, 2:44 AM