[all-commits] [llvm/llvm-project] 9d963b: [mlir][OpenMP] Change the definition of omp.private

Tom Eccles via All-commits all-commits at lists.llvm.org
Wed Jan 22 13:32:12 PST 2025 

  Branch: refs/heads/users/tblah/delayed-task-exec-1
  Home:   https://github.com/llvm/llvm-project
  Commit: 9d963b7be234331d6d7e3ec79be518ea7a90ba88
      https://github.com/llvm/llvm-project/commit/9d963b7be234331d6d7e3ec79be518ea7a90ba88
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Optimizer/CodeGen/CodeGenOpenMP.cpp
    M flang/test/Analysis/AliasAnalysis/alias-analysis-omp-private-allocatable.mlir
    M flang/test/Analysis/AliasAnalysis/alias-analysis-omp-teams-distribute-private-ptr.mlir
    M flang/test/Analysis/AliasAnalysis/alias-analysis-omp-teams-distribute-private.mlir
    M flang/test/Fir/boxproc-openmp.fir
    M flang/test/HLFIR/opt-variable-assign-omp.fir
    M flang/test/Integration/OpenMP/parallel-private-reduction-worstcase.f90
    M flang/test/Integration/OpenMP/private-global.f90
    M flang/test/Lower/OpenMP/DelayedPrivatization/distribute-standalone-private.f90
    M flang/test/Lower/OpenMP/DelayedPrivatization/target-private-allocatable.f90
    M flang/test/Lower/OpenMP/DelayedPrivatization/target-private-simple.f90
    M flang/test/Lower/OpenMP/DelayedPrivatization/wsloop.f90
    M flang/test/Lower/OpenMP/cfg-conversion-omp.private.f90
    M flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90
    M flang/test/Lower/OpenMP/delayed-privatization-allocatable-firstprivate.f90
    M flang/test/Lower/OpenMP/delayed-privatization-allocatable-private.f90
    M flang/test/Lower/OpenMP/delayed-privatization-array.f90
    M flang/test/Lower/OpenMP/delayed-privatization-character-array.f90
    M flang/test/Lower/OpenMP/delayed-privatization-firstprivate.f90
    M flang/test/Lower/OpenMP/delayed-privatization-private-firstprivate.f90
    M flang/test/Lower/OpenMP/delayed-privatization-private.f90
    M flang/test/Lower/OpenMP/delayed-privatization-reduction-byref.f90
    M flang/test/Lower/OpenMP/delayed-privatization-reduction.f90
    M flang/test/Lower/OpenMP/implicit-dsa.f90
    M flang/test/Lower/OpenMP/loop-directive.f90
    M flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90
    M flang/test/Lower/OpenMP/same_var_first_lastprivate.f90
    M flang/test/Lower/OpenMP/simd.f90
    M flang/test/Lower/OpenMP/task2.f90
    M flang/test/Transforms/generic-loop-rewriting.mlir
    M flang/test/Transforms/omp-maps-for-privatized-symbols.fir
    M mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
    M mlir/lib/Conversion/OpenMPToLLVM/OpenMPToLLVM.cpp
    M mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
    M mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
    M mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
    M mlir/test/Dialect/OpenMP/invalid.mlir
    M mlir/test/Dialect/OpenMP/ops.mlir
    M mlir/test/Target/LLVMIR/openmp-firstprivate.mlir
    M mlir/test/Target/LLVMIR/openmp-llvm.mlir
    M mlir/test/Target/LLVMIR/openmp-omp.private-dealloc.mlir
    M mlir/test/Target/LLVMIR/openmp-private.mlir
    M mlir/test/Target/LLVMIR/openmp-simd-private.mlir
    M mlir/test/Target/LLVMIR/openmp-target-multiple-private.mlir
    M mlir/test/Target/LLVMIR/openmp-target-private-allocatable.mlir
    M mlir/test/Target/LLVMIR/openmp-target-private.mlir
    M mlir/test/Target/LLVMIR/openmp-target-simd-on_device.mlir
    M mlir/test/Target/LLVMIR/openmp-todo.mlir
    M mlir/test/Target/LLVMIR/openmp-wsloop-private-cond_br.mlir
    M mlir/test/Target/LLVMIR/openmp-wsloop-private.mlir

  Log Message:
  -----------
  [mlir][OpenMP] Change the definition of omp.private

The intention of this work is to give MLIR->LLVMIR conversion freedom to
control how the private variable is allocated so that it can be
allocated on the stack in ordinary cases or as part of a structure used
to give closure context for tasks which might outlive the current stack
frame. See RFC:
https://discourse.llvm.org/t/rfc-openmp-supporting-delayed-task-execution-with-firstprivate-variables/83084

In flang, before this patch we hit a TODO with the same wording when
generating the copy region for firstprivate polymorphic variables. After
this patch the box-like fir.class is passed by reference into the copy
region, leading to a different path that didn't hit that old TODO but
the generated code still didn't work so I added a new TODO in
DataSharingProcessor.

---

Please read mlir changes first and then flang changes.

In flang lowering I box up all arrays and pass the boxes by reference so
that the existing code for reduction init and dealloc regions can be
shared.

The TODOs for pointers, derived types, characters etc will be resolved
in later patches in this same series (to be squashed into this one). I
separated it to make it easier to review.

Assumed rank was already broken before this patch.
I can't find any mention of "assumed rank" in the openmp standard so
I guess it is not prohibited.

Other than the omp.private operation definition changes, the test changes
are mostly down to slightly different codegen from re-using the reduction
init region. That code is already well tested so I didn't want to change it.


  Commit: ca55879fe79351ae9bedc45f5438ae34a94e2d2e
      https://github.com/llvm/llvm-project/commit/ca55879fe79351ae9bedc45f5438ae34a94e2d2e
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    A flang/test/Lower/OpenMP/delayed-privatization-lastprivate-of-private.f90

  Log Message:
  -----------
  Fix bug when used with lastprivate

Arrays are boxed and those boxed arrays are passed by reference. This
can cause problems if we try to perform eager privatization (e.g.
lastprivate) on a variable already privatized using delayed
privatization. Work around this by loading the reference to the box.


  Commit: ee3539dddf29f811f41baf606829dea66d7486cb
      https://github.com/llvm/llvm-project/commit/ee3539dddf29f811f41baf606829dea66d7486cb
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/test/Lower/OpenMP/delayed-privatization-character.f90
    M flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90
    M flang/test/Lower/OpenMP/parallel-private-clause-str.f90
    M flang/test/Lower/OpenMP/private-commonblock.f90

  Log Message:
  -----------
  Support char types

The way boxchars are handled isn't ideal, but !fir.ref<!fir.boxchar<>>
seems to violate a lot of assumptions in the wider ecosystem of (hl)fir
helpers making it difficult to generate a copy region. I suspect
!fir.ref<!fir.boxchar<>> is not supposed to work (it looks like a
mutable character box, which isn't possible because a boxchar should be
an SSA value). Fixing this would be a big change beyond the scope of
this already too large PR.


  Commit: 2eb72ceb4528778bc0f15f73749a5c89d4d225f0
      https://github.com/llvm/llvm-project/commit/2eb72ceb4528778bc0f15f73749a5c89d4d225f0
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Optimizer/OpenMP/MapsForPrivatizedSymbols.cpp
    M flang/test/Lower/OpenMP/DelayedPrivatization/target-private-multiple-variables.f90

  Log Message:
  -----------
  Fix omp target maps for privatized symbols


  Commit: dc820bb1d9a7f32414a73248d702eeea956641f2
      https://github.com/llvm/llvm-project/commit/dc820bb1d9a7f32414a73248d702eeea956641f2
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.h
    M flang/test/Lower/OpenMP/DelayedPrivatization/equivalence.f90
    M flang/test/Lower/OpenMP/delayed-privatization-pointer.f90

  Log Message:
  -----------
  Support pointers

The copy region codegen hasn't changed as a result of this patch series.
However I think there is a bug in the copy region generated in
equivalence.f90. This patch series is already too big so I won't change
the copy region here.


  Commit: 870af174eee0f862f515862d198b4ed299780de4
      https://github.com/llvm/llvm-project/commit/870af174eee0f862f515862d198b4ed299780de4
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/test/Lower/OpenMP/delayed-privatization-allocatable-array.f90
    M flang/test/Lower/OpenMP/parallel-reduction-allocatable-array.f90
    M flang/test/Lower/OpenMP/parallel-reduction-pointer-array.f90
    A flang/test/Lower/OpenMP/pointer-to-array.f90
    M flang/test/Lower/OpenMP/wsloop-reduction-allocatable-array-minmax.f90

  Log Message:
  -----------
  Fix crash lowering fir::EmboxOp whithout shape

This only affects POINTERs (which should not be initialized) and NULL
allocatables so the actual contents of the shape doesn't matter. This is
just so the embox operation is converted to LLVMIR correctly.


  Commit: 0015bb3b817f557914a610659a55569b5eb14430
      https://github.com/llvm/llvm-project/commit/0015bb3b817f557914a610659a55569b5eb14430
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.h
    M flang/test/Integration/OpenMP/copyprivate.f90
    M flang/test/Lower/OpenMP/default-clause-byref.f90
    M flang/test/Lower/OpenMP/delayed-privatization-default-init.f90
    M flang/test/Lower/OpenMP/firstprivate-alloc-comp.f90
    M flang/test/Lower/OpenMP/parallel-private-clause.f90

  Log Message:
  -----------
  Support derived types

There are three cases handled:
- Boxed (maybe allocatable or pointer) scalar derived types
- Boxed (maybe allocatable or pointer) arrays of derived types
- Unboxed scalar derived types

Currently I support both boxed and unboxed derived types because unboxed
derived types aren't hlfir::Entities so I worry there could be cases
where they are in fact boxed.

The changes to parallel-private-clause.f90 are because of the arrays
becoming boxed. I re-organised the test a bit because the CHECK-DAGs
were matching completely the wrong lines outside of the parallel region.


  Commit: 4d52dde46c2e121659415afa9d5cd0501aadb38c
      https://github.com/llvm/llvm-project/commit/4d52dde46c2e121659415afa9d5cd0501aadb38c
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Lower/OpenMP/DataSharingProcessor.h
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.h
    M flang/lib/Lower/OpenMP/ReductionProcessor.cpp
    M flang/test/Lower/OpenMP/derived-type-allocatable.f90
    M flang/test/Lower/OpenMP/private-derived-type.f90

  Log Message:
  -----------
  Implement lupori's derive type initialization patch

For reference see both
https://github.com/llvm/llvm-project/pull/120295 and
https://github.com/llvm/llvm-project/pull/121808

This changes the barrier logic.
See discussion here: https://github.com/llvm/llvm-project/pull/120295/files#r1910663473


  Commit: eec1a1cc0e63e3410da2619cbae03fc5d496244d
      https://github.com/llvm/llvm-project/commit/eec1a1cc0e63e3410da2619cbae03fc5d496244d
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.h
    M flang/lib/Lower/OpenMP/ReductionProcessor.cpp
    M flang/lib/Lower/OpenMP/ReductionProcessor.h

  Log Message:
  -----------
  Deallocate derived types

This will deallocate in exactly the same cases as the old implementation
before this series. I expect some cases are missed but I want to avoid
functional changes wherever possible in this (already too big) patch
series.


  Commit: 863ba4552485815a0d494d08690a4226ee5ca13d
      https://github.com/llvm/llvm-project/commit/863ba4552485815a0d494d08690a4226ee5ca13d
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/test/Lower/OpenMP/derived-type-allocatable.f90

  Log Message:
  -----------
  Support arrays of polymorphic types


  Commit: 04e6c516555869dc60f68c694367d1c54d0c9eba
      https://github.com/llvm/llvm-project/commit/04e6c516555869dc60f68c694367d1c54d0c9eba
  Author: Tom Eccles <tom.eccles at arm.com>
  Date:   2025-01-22 (Wed, 22 Jan 2025)

  Changed paths:
    M flang/lib/Lower/OpenMP/PrivateReductionUtils.cpp
    M flang/test/Lower/OpenMP/copyprivate.f90
    M flang/test/Lower/OpenMP/delayed-privatization-pointer.f90

  Log Message:
  -----------
  Fix compiler crash for unnesecarry len params


Compare: https://github.com/llvm/llvm-project/compare/9d963b7be234%5E...04e6c5165558

To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications

More information about the All-commits mailing list