Skip to content

Commit

Permalink
[Flang][OpenMP][MLIR] Remove deletion of unused declare target global…
Browse files Browse the repository at this point in the history
… after use replacement (#67762)

At the moment, for device a reference pointer is generated in place of
the original declare target global value, this reference pointer is the
pointer that actually receives the data. In Clang the original global
value isn't generated for device, just the reference pointer.

Unfortunately for Flang/MLIR this is currently not the case, as the
declare target attribute is processed after the creation of the global
so we end up with a dead global on device effectively after rewriting
its uses to the new device reference pointer.

It appears I was a little overzealous with the deletion of the declare
target globals for device. The current method breaks in-cases where the
same declare target global is used across two target regions (added a
runtime reproduced in the patch). As it'll effectively delete it before
the second target gets a chance to be written to LLVM IR and have it's
uses rewritten .

I'd like to remove this deletion as the dead global isn't breaking any
code and will likely be removed in later dead code elimination passes,
perhaps a little too heavy handed with the original approach.
  • Loading branch information
agozillon committed Oct 3, 2023
1 parent bc0c178 commit 1482106
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1989,15 +1989,6 @@ handleDeclareTargetMapVar(llvm::ArrayRef<Value> mapOperands,
user->replaceUsesOfWith(mapOpValue, load);
}
}

// A global has already been generated by this stage for device
// that's now dead after the remapping, we can remove it now,
// as we have replaced all usages with the new ref pointer.
if (llvm::GlobalValue *unusedGlobalVal =
dyn_cast<llvm::GlobalValue>(mapOpValue)) {
unusedGlobalVal->dropAllReferences();
unusedGlobalVal->eraseFromParent();
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
! Offloading test with two target regions mapping the same
! declare target Fortran array and writing some values to
! it before checking the host correctly receives the
! correct updates made on the device.
! REQUIRES: flang, amdgcn-amd-amdhsa
! UNSUPPORTED: nvptx64-nvidia-cuda
! UNSUPPORTED: nvptx64-nvidia-cuda-LTO
! UNSUPPORTED: aarch64-unknown-linux-gnu
! UNSUPPORTED: aarch64-unknown-linux-gnu-LTO
! UNSUPPORTED: x86_64-pc-linux-gnu
! UNSUPPORTED: x86_64-pc-linux-gnu-LTO

! RUN: %libomptarget-compile-fortran-run-and-check-generic
module test_0
implicit none
integer :: sp(10) = (/0,0,0,0,0,0,0,0,0,0/)
!$omp declare target link(sp)
end module test_0

program main
use test_0
integer :: i = 1
integer :: j = 11

!$omp target map(tofrom:sp) map(to: i, j)
do while (i <= j)
sp(i) = i;
i = i + 1
end do
!$omp end target

!$omp target map(tofrom:sp) map(to: i, j)
do while (i <= j)
sp(i) = sp(i) + i;
i = i + 1
end do
!$omp end target

print *, sp(:)

end program

! CHECK: 2 4 6 8 10 12 14 16 18 20

0 comments on commit 1482106

Please sign in to comment.