[Flang][OpenMP] Fix allocating arrays with size intrinisic #225
Conversation
flang/lib/Lower/DirectivesCommon.h
Outdated
| inline AddrAndBoundsInfo getDataOperandBaseAddr(fir::FirOpBuilder &builder, | ||
| mlir::Value symAddr, | ||
| bool isOptional, | ||
| mlir::Location loc) { |
There was a problem hiding this comment.
Spurious change of formatting. Please undo.
There was a problem hiding this comment.
Thank you will address that in an update in a little bit!
| // which comes with a fairly large overhead comparatively. We could be | ||
| // more robust about this and check using a BackWardsSlice to see if we | ||
| // run the risk of mapping a box. |
There was a problem hiding this comment.
Would this warrant an assert or something to make this explicitly visible should a code cause this to happen?
There was a problem hiding this comment.
I am a bit hesitant to add an assert, as it isn't an issue of correctness (at least it shouldn't be now after this patch and some testing), as it will work, it's just a bit more of an egregious copy now than it was before as the mappings gotten a little more complicated, so avoiding where possible is nice. Basically don't want to assert for perfectly valid and working code! It would be nice to be able to add performance warnings for these kinds of cases as opposed to flat ICEs/asserts, but I don't think flang has infrastructure for this sadly
Attempt to address the following example from causing an assert or ICE:
subroutine test(a)
implicit none
integer :: i
real(kind=real64), dimension(:) :: a
real(kind=real64), dimension(size(a, 1)) :: b
!$omp target map(tofrom: b)
do i = 1, 10
b(i) = i
end do
!$omp end target
end subroutine
Where we utilise a Fortran intrinsic (size) to calculate the size of allocatable arrays and then map it to device.
Borrowing some of Kareem Ergawy's current work to disentangle bounds generation from the semantic/PFT information.
Co-author: Kareem Ergawy : kareem.ergawy@amd.com
agozillon
force-pushed
the
size-intrinsic-fix
branch
from
68a9311 to
d818282
Compare
Attempt to address the following example from causing an assert or ICE:
subroutine test(a)
implicit none
integer :: i
real(kind=real64), dimension(:) :: a
real(kind=real64), dimension(size(a, 1)) :: b
!$omp target map(tofrom: b)
do i = 1, 10
b(i) = i
end do
!$omp end target
end subroutine
Where we utilise a Fortran intrinsic (size) to calculate the size of allocatable arrays and then map it to device.
Borrowing some of Kareem Ergawy's current work to disentangle bounds generation from the semantic/PFT information.
Co-author: Kareem Ergawy : kareem.ergawy@amd.com