[all-commits] [llvm/llvm-project] ccb5d2: [OpenMP][FIX] Avoid a race between initialization ...

Tue Nov 2 21:23:02 PDT 2021

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ccb5d2726a8bc0092857f9e69963e872d78cd4d8
      https://github.com/llvm/llvm-project/commit/ccb5d2726a8bc0092857f9e69963e872d78cd4d8
  Author: Johannes Doerfert <johannes at jdoerfert.de>
  Date:   2021-11-02 (Tue, 02 Nov 2021)

  Changed paths:
    M openmp/libomptarget/DeviceRTL/include/Mapping.h
    M openmp/libomptarget/DeviceRTL/src/Kernel.cpp
    M openmp/libomptarget/DeviceRTL/src/Mapping.cpp
    M openmp/libomptarget/DeviceRTL/src/State.cpp

  Log Message:
  -----------
  [OpenMP][FIX] Avoid a race between initialization and first state reads

When we pick state 0 to initialize state but thread N is going to be the
"main thread", in generic mode, we would require extra synchronization.
Instead, we should pick the main thread to initialize state in generic
mode and any thread in SPMD mode.

Reviewed By: tianshilei1992

Differential Revision: https://reviews.llvm.org/D112874

  Commit: e6e440ae5f8de23bea20ecf03394d46cbe5c11f8
      https://github.com/llvm/llvm-project/commit/e6e440ae5f8de23bea20ecf03394d46cbe5c11f8
  Author: Johannes Doerfert <johannes at jdoerfert.de>
  Date:   2021-11-02 (Tue, 02 Nov 2021)

  Changed paths:
    M llvm/lib/Transforms/IPO/OpenMPOpt.cpp

  Log Message:
  -----------
  [OpenMP][FIX] Ensure guarding uses proper global name

Global symbols cannot have any name so we need to sanitize the string
first. Also remove an assertion that is not actually necessary nor
true in general.

Reviewed By: ggeorgakoudis

Differential Revision: https://reviews.llvm.org/D112892

  Commit: c690c1c977ffb25c3334431604ed771cb6b43b72
      https://github.com/llvm/llvm-project/commit/c690c1c977ffb25c3334431604ed771cb6b43b72
  Author: Johannes Doerfert <johannes at jdoerfert.de>
  Date:   2021-11-02 (Tue, 02 Nov 2021)

  Changed paths:
    M llvm/include/llvm/IR/IntrinsicsNVVM.td
    M llvm/test/Transforms/OpenMP/replace_globalization.ll

  Log Message:
  -----------
  [NVVM] Update intrinsic definitions to include more attributes

A lot of NVVM intrinsics can use the default intrinsic attributes (e.g.,
nosync, nofree, ...) as well as `speculatable`. The latter is important
if we want to recompute intrinsics results instead of communicating them
via memory.

I did use default attributes for almost all `readnone` attributes but
speculatable only where I had reasonable confidence they cannot
experience UB. That said, someone should double check.

TODO: There seem to be various intrinsics marked `Commutative` which
      should not, e.g., fma and div.

Reviewed By: tra

Differential Revision: https://reviews.llvm.org/D109987

  Commit: 73720c8059cfcce12f0cc5b7e6ff2e4b635a9a61
      https://github.com/llvm/llvm-project/commit/73720c8059cfcce12f0cc5b7e6ff2e4b635a9a61
  Author: Johannes Doerfert <johannes at jdoerfert.de>
  Date:   2021-11-02 (Tue, 02 Nov 2021)

  Changed paths:
    M llvm/include/llvm/Frontend/OpenMP/OMPKinds.def
    M llvm/lib/Transforms/IPO/OpenMPOpt.cpp
    M llvm/test/Transforms/OpenMP/custom_state_machines.ll
    M llvm/test/Transforms/OpenMP/spmdization.ll
    M llvm/test/Transforms/OpenMP/spmdization_guarding.ll
    M openmp/libomptarget/DeviceRTL/include/Interface.h
    M openmp/libomptarget/DeviceRTL/src/Synchronization.cpp
    M openmp/libomptarget/DeviceRTL/src/Utils.cpp
    M openmp/libomptarget/deviceRTLs/common/src/sync.cu
    M openmp/libomptarget/deviceRTLs/interface.h

  Log Message:
  -----------
  [OpenMP][FIX] Introduce and use a simple generic-mode barrier

Before we had aligned barriers the `__kmpc_barrier_simple_spmd` was
OK to be used in the custom state machine. Now that SPMD barriers are
assumed to be aligned we need to use a "generic" barrier in places
that are not aligned.

Reviewed By: tianshilei1992

Differential Revision: https://reviews.llvm.org/D112893

  Commit: d61aac76bf9009317ef15b1fc38a98a808ce9b19
      https://github.com/llvm/llvm-project/commit/d61aac76bf9009317ef15b1fc38a98a808ce9b19
  Author: Johannes Doerfert <johannes at jdoerfert.de>
  Date:   2021-11-02 (Tue, 02 Nov 2021)

  Changed paths:
    M llvm/lib/Transforms/IPO/OpenMPOpt.cpp
    M llvm/test/Transforms/OpenMP/always_inline_device.ll
    M llvm/test/Transforms/OpenMP/get_hardware_num_threads_in_block_fold.ll

  Log Message:
  -----------
  [OpenMP][FIX] Do not signal SPMD-mode but then keep generic-mode

If we assume SPMD-mode during the fixpoint iteration we have to execute
the kernel in SPMD-mode. If we change our mind during manifest there is
the chance of a mismatch between the simplification, e.g., of
`__kmpc_is_spmd_exec_mode` calls, and the execution mode. This problem
was introduced in D109438.

This patch is compromise to resolve the problem purely in OpenMP-opt
while trying to keep the benefits of D109438 around. This might not
always work, see `get_hardware_num_threads_in_block_fold` but it often
does. At the same time we do keep value specialization and execution
mode in sync.

Proper solutions to this problem should be considered. I believe a new
execution mode is the easiest way forward (Singleton-SPMD).
Alternatively, SPMD-mode execution can be used with a way to provide a
new thread_limit (here 1) to the runtime. This is more general and could
be useful if we see `num_threads` clauses or workshared loops with small
trip counts in the kernel. In either proposal we need to disable the
guarding for the kernel (which was the motivation for D109438).

Reviewed By: jhuber6

Differential Revision: https://reviews.llvm.org/D112894

Compare: https://github.com/llvm/llvm-project/compare/fbe61fb0aa23...d61aac76bf90