[flang-commits] [flang] [flang][OpenMP] fix predetermined privatization inside section (PR #138159)

[Tom Eccles via flang-commits]

https://github.com/tblah created https://github.com/llvm/llvm-project/pull/138159

This now produces code equivalent to if there was an explicit private clause on the SECTIONS construct.

The problem was that each SECTION construct got its own DSP, which tried to privatize the same symbol for that SECTION. Privatization for SECTION(S) happens on the outer SECTION construct and so the outer construct's DSP should be shared.

Fixes #135108

>From 91aa3bb1a0cdb61ff5be27440f88e29e1cb78455 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Thu, 1 May 2025 10:34:28 +0000
Subject: [PATCH] [flang][OpenMP] fix predetermined privatization inside
 section

This now produces code equivalent to if there was an explicit private
clause on the SECTIONS construct.

The problem was that each SECTION construct got its own DSP, which tried
to privatize the same symbol for that SECTION. Privatization for
SECTION(S) happens on the outer SECTION construct and so the outer
construct's DSP should be shared.

Fixes #135108
---
 flang/lib/Lower/OpenMP/OpenMP.cpp             | 14 ++++++--
 .../OpenMP/sections-predetermined-private.f90 | 34 +++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 flang/test/Lower/OpenMP/sections-predetermined-private.f90

diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index f099028c23323..ae28d5ddd1564 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -1057,6 +1057,11 @@ struct OpWithBodyGenInfo {
     return *this;
   }
 
+  OpWithBodyGenInfo &setSkipDspStep2(bool value) {
+    skipDspStep2 = value;
+    return *this;
+  }
+
   OpWithBodyGenInfo &setEntryBlockArgs(const EntryBlockArgs *value) {
     blockArgs = value;
     return *this;
@@ -1088,6 +1093,8 @@ struct OpWithBodyGenInfo {
   const List<Clause> *clauses = nullptr;
   /// [in] if provided, processes the construct's data-sharing attributes.
   DataSharingProcessor *dsp = nullptr;
+  /// [in] if true, skip DataSharingProcessor::processStep2
+  bool skipDspStep2 = false;
   /// [in] if provided, it is used to create the op's region entry block. It is
   /// overriden when a \see genRegionEntryCB is provided. This is only valid for
   /// operations implementing the \see mlir::omp::BlockArgOpenMPOpInterface.
@@ -1240,7 +1247,7 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
     // loop (this may not make sense in production code, but a user could
     // write that and we should handle it).
     firOpBuilder.setInsertionPoint(term);
-    if (privatize) {
+    if (privatize && !info.skipDspStep2) {
       // DataSharingProcessor::processStep2() may create operations before/after
       // the one passed as argument. We need to treat loop wrappers and their
       // nested loop as a unit, so we need to pass the bottom level wrapper (if
@@ -2162,7 +2169,10 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
         OpWithBodyGenInfo(converter, symTable, semaCtx, loc, nestedEval,
                           llvm::omp::Directive::OMPD_section)
             .setClauses(&sectionQueue.begin()->clauses)
-            .setEntryBlockArgs(&args),
+            .setEntryBlockArgs(&args)
+            .setDataSharingProcessor(&dsp)
+            // lastprivate is handled differently for SECTIONS, see below
+            .setSkipDspStep2(true),
         sectionQueue, sectionQueue.begin());
   }
 
diff --git a/flang/test/Lower/OpenMP/sections-predetermined-private.f90 b/flang/test/Lower/OpenMP/sections-predetermined-private.f90
new file mode 100644
index 0000000000000..9c2e2e127aa78
--- /dev/null
+++ b/flang/test/Lower/OpenMP/sections-predetermined-private.f90
@@ -0,0 +1,34 @@
+! RUN: %flang_fc1 -fopenmp -emit-hlfir -o - %s | FileCheck %s
+
+!$omp parallel sections
+!$omp section
+    do i = 1, 2
+    end do
+!$omp section
+    do i = 1, 2
+    end do
+!$omp end parallel sections
+end
+! CHECK-LABEL:   func.func @_QQmain() {
+! CHECK:           omp.parallel {
+! CHECK:             %[[VAL_3:.*]] = fir.alloca i32 {bindc_name = "i", pinned}
+! CHECK:             %[[VAL_4:.*]]:2 = hlfir.declare %[[VAL_3]] {uniq_name = "_QFEi"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>)
+! CHECK:             omp.sections {
+! CHECK:               omp.section {
+! CHECK:                 %[[VAL_11:.*]]:2 = fir.do_loop %[[VAL_12:.*]] = %{{.*}} to %{{.*}} step %{{.*}} iter_args(%{{.*}} = %{{.*}} -> (index, i32) {
+! CHECK:                 }
+! CHECK:                 fir.store %[[VAL_11]]#1 to %[[VAL_4]]#0 : !fir.ref<i32>
+! CHECK:                 omp.terminator
+! CHECK:               }
+! CHECK:               omp.section {
+! CHECK:                 %[[VAL_25:.*]]:2 = fir.do_loop %[[VAL_26:.*]] = %{{.*}} to %{{.*}} step %{{.*}} iter_args(%{{.*}} = %{{.*}}) -> (index, i32) {
+! CHECK:                 }
+! CHECK:                 fir.store %[[VAL_25]]#1 to %[[VAL_4]]#0 : !fir.ref<i32>
+! CHECK:                 omp.terminator
+! CHECK:               }
+! CHECK:               omp.terminator
+! CHECK:             }
+! CHECK:             omp.terminator
+! CHECK:           }
+! CHECK:           return
+! CHECK:         }