[flang][OpenMP] Initial support the lowering of copyin clause

[peixin]

Summary

This supports the lowering of copyin clause initially. The pointer,
allocatable, common block, polymorphic varaibles will be supported
later.

This also includes the following changes:

1. Resolve the COPYIN clause and make the entity as host associated.

2. Fix collectSymbolSet by adding one option to control collecting the symbol itself or ultimate symbol of it so that it can be used explicitly differentiate the host and associated variables in host-association.

3. Add one helper function lookupOneLevelUpSymbol to differentiate the usage of host and associated variables explicitly. The previous lowering of firstprivate depends on the order of createHostAssociateVarClone and lookupSymbol of host symbol. With this fix, this dependence is removed.

4. Reuse copyHostAssociateVar for copying operation of COPYIN clause.

[NimishMishra]

This LGTM. Explicit differentiation of host and associated variables in host-association seems ok to me.

A few comments.

flang/lib/Lower/Bridge.cpp

522–525

This piece of change is something that I use in the default clause lowering as well. I will reuse it from your patch here. Thanks for this

flang/test/Lower/OpenMP/copyin.f90

3

@kiranchandramohan commented on D125668 to include flang driver tests also. Do we start doing it with all of our patches, or was it a one off thing? Maybe @kiranchandramohan can suggest better.

175

I do not understand the double omp.threadprivate %[[VAL_0]] here. It also seems like VAL_2 is not used here after. Do we expect the separate section to have their own separate x7?

[peixin]

flang/lib/Lower/Bridge.cpp

522–525

Please try it. You can also expand this function for default clause if necessary.

flang/test/Lower/OpenMP/copyin.f90

3

OK. I agree with @kiranchandramohan . For new patches, let us add two RUNs with bbc and flang-new. Maybe he will refactoring all the test cases when finishing the upstreaming.

175

The double omp.threadprivate %[[VAL_0]] is there since two section directives are there. I think this is the problem of not good organization of current constructs and clauses in OpenMP.cpp. It can be addressed by either refactoring OpenMP.cpp or removing the dead code in O3 somewhere.

> Do we expect the separate section to have their own separate x7?

I don't think so. The copyin clause is not allowed in sections construct. For this combined construct, copyin is for parallel construct. So, two x7 share one storage.

[peixin]

Fix the double threadprivatization.

[peixin]

flang/lib/Lower/OpenMP.cpp

174

@NimishMishra Please notice that using original symbol (aka. associated symbol) may cause multiple privatization. I changed the collectSymbolSet so to make it more generous. Then you can use it for either host or associated symbols explicitly.

[peixin]

rebase

[NimishMishra]

flang/lib/Lower/OpenMP.cpp

174

That's great! Thank you.

[schweitz]

flang/include/flang/Lower/AbstractConverter.h

110

Is there a more descriptive term than "original" for these symbols? It's not entirely clear what is meant exactly.

[peixin]

Address the comments from @schweitz .

[peixin]

flang/include/flang/Lower/AbstractConverter.h

110

I reverse the logic of the boolean variable and use the ultimate symbol by default so to make it be consistent with collectHostAssociatedVariables. Is it OK to you now?

[kiranchandramohan]

No change requested: An alternative method would be to let the privatisation/threadprivatisation function return the host symbol and then use that for making the copy. This might need some rearrangement of the code but will not require the separate host lookup function. Have you considered this?

flang/lib/Lower/OpenMP.cpp

176

Nit: Is else required?

[peixin]

Remove the else.

[peixin]

In D127468#3595789, @kiranchandramohan wrote:

No change requested: An alternative method would be to let the privatisation/threadprivatisation function return the host symbol and then use that for making the copy. This might need some rearrangement of the code but will not require the separate host lookup function. Have you considered this?

Using the function return value or pointer argument storing the host symbol can solve the problem. But it is still kind of not readable. I add the helper function and use the current code for another purpose: to make the symbol mapping explicitly shown to the following developers. When I first read and write the code for private/firstprivate/threadprivate/copyin constructs/clauses, I am really confused to use the lookupSymbol since the symbol mapping is not shown directly. With both lookupHostSymbol and lookupSymbol functions, it is easier for the following developers who wants to reuse the copyHostAssociateVar to know what they are using. And after reading lookupHostSymbol and lookupSymbol, it is easy to know the symbols are organized into different symbol map stack. What do you think?

flang/lib/Lower/OpenMP.cpp

176

Thanks. Fixed.

[kiranchandramohan]

In D127468#3596166, @peixin wrote:

In D127468#3595789, @kiranchandramohan wrote:

No change requested: An alternative method would be to let the privatisation/threadprivatisation function return the host symbol and then use that for making the copy. This might need some rearrangement of the code but will not require the separate host lookup function. Have you considered this?

Using the function return value or pointer argument storing the host symbol can solve the problem. But it is still kind of not readable. I add the helper function and use the current code for another purpose: to make the symbol mapping explicitly shown to the following developers. When I first read and write the code for private/firstprivate/threadprivate/copyin constructs/clauses, I am really confused to use the lookupSymbol since the symbol mapping is not shown directly. With both lookupHostSymbol and lookupSymbol functions, it is easier for the following developers who wants to reuse the copyHostAssociateVar to know what they are using. And after reading lookupHostSymbol and lookupSymbol, it is easy to know the symbols are organized into different symbol map stack. What do you think?

When this was initially implemented we had separate bindings for the host symbol and the associated symbols. Subsequently, @schweitz refactored code to simplify the lowering and used the ultimate symbol always. This meant that the SymbolBox/ExtendedValue used are mapped with the same key value (the ultimate symbol) for both the host and the associated symbol.
I have the following issues/questions.
-> I don't know whether skipping a level of the map is desirable (or always correct) for accessing the host symbol's binding.
-> Also skipping a level does not necessarily mean we are getting a host value. Whereas if we use the hostSymbolBox that was used for privatising/threadprivatising, we are guaranteed that we have the host Symbol's binding.
-> Also, this function (lookupHostSymbol) has to be strictly used after creating a binding for the associated symbol.

[peixin]

-> I don't know whether skipping a level of the map is desirable (or always correct) for accessing the host symbol's binding.

@schweitz Is this OK with you?

-> Also skipping a level does not necessarily mean we are getting a host value. Whereas if we use the hostSymbolBox that was used for privatising/threadprivatising, we are guaranteed that we have the host Symbol's binding.

Yes, the prerequisite of getting a host value using lookupHostSymbol is calling it in the associated symbol's stack. Otherwise, it returns null symbol box, which is expected.

The previous copyHostAssociateVar actually do two things, create the host-associated value and copy host to associated value. I move the creation operation out since I want to use it for copyin clause. Then, hostSymbolBox cannot guarantee getting the host symbol's binding since the associated level symbol map stack already have the symbol binding.

-> Also, this function (lookupHostSymbol) has to be strictly used after creating a binding for the associated symbol.

No need. This function has nothing to do with the binding in the associated level. If calling this function in the associated level symbol map stack when there is no binding for the associated symbol, you can get the host value. The difference between lookupSymbol and lookupHostSymbol for this case (has not created the binding for the associated symbol) is that lookupSymbol does not find the symbol box in the associated level symbol map stack and lookupHostSymbol skip the associated level symbol map stack.

[peixin]

When this was initially implemented we had separate bindings for the host symbol and the associated symbols. Subsequently, @schweitz refactored code to simplify the lowering and used the ultimate symbol always. This meant that the SymbolBox/ExtendedValue used are mapped with the same key value (the ultimate symbol) for both the host and the associated symbol.

I think there is some difference in the host association between Fortran and OpenMP. In Fortran, it is taken as one argument of procedure. In OpenMP, we use the host and associated symbols in the same program unit.

[kiranchandramohan]

In D127468#3596469, @peixin wrote:

-> Also, this function (lookupHostSymbol) has to be strictly used after creating a binding for the associated symbol.

No need. This function has nothing to do with the binding in the associated level. If calling this function in the associated level symbol map stack when there is no binding for the associated symbol, you can get the host value. The difference between lookupSymbol and lookupHostSymbol for this case (has not created the binding for the associated symbol) is that lookupSymbol does not find the symbol box in the associated level symbol map stack and lookupHostSymbol skip the associated level symbol map stack.

What I mean here is that if you skip one level, there is a possibility that you can pop out the map containing the entry for the host version.

[peixin]

Using the function return value or pointer argument storing the host symbol can solve the problem.

Sorry, I did not explain it correctly. Here, it should be one vector of the symbol box of the host symbol instead of the host symbol since the host symbol can be the common block (D127215). It would be like the following:

diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6caea151c20e..c759276a2f06 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -422,14 +422,16 @@ public:
         llvm::None);
   }
 
-  bool createHostAssociateVarClone(
+  llvm::SmallVector<Fortran::lower::SymbolBox> createHostAssociateVarClone(
       const Fortran::semantics::Symbol &sym) override final {
+    llvm::SmallVector<Fortran::lower::SymbolBox> sbs;
     mlir::Location loc = genLocation(sym.name());
     mlir::Type symType = genType(sym);
     const auto *details = sym.detailsIf<Fortran::semantics::HostAssocDetails>();
     assert(details && "No host-association found");
     const Fortran::semantics::Symbol &hsym = details->symbol();
     Fortran::lower::SymbolBox hsb = lookupSymbol(hsym);
+    sbs.emplace_back(hsb);
 
     auto allocate = [&](llvm::ArrayRef<mlir::Value> shape,
                         llvm::ArrayRef<mlir::Value> typeParams) -> mlir::Value {
@@ -472,7 +474,8 @@ public:
           return fir::substBase(hexv, temp);
         });
 
-    return bindIfNewSymbol(sym, exv);
+    assert(bindIfNewSymbol(sym, exv) && "failed binding");
+    return sbs;
   }
 
   void

In addition, the implementation of copyHostAssociateVar for common block will become more complex. You can check the implementation of private clause for common block. The easy extension will handle it and the structure is clear.

What I mean here is that if you skip one level, there is a possibility that you can pop out the map containing the entry for the host version.

If using this function is in the associated level symbol map stack, there is no such possibility. If using this function is in the host level symbol map stack, it does not skip the host level and returns one null symbol box, for which case, you should use lookupSymbol.

Once creating one symbol-value binding for the host associated symbol, there is no helper function to get the host symbol value. One function supporting it can help us make the code simple and readable.

[kiranchandramohan]

In D127468#3596533, @kiranchandramohan wrote:

-> Also, this function (lookupHostSymbol) has to be strictly used after creating a binding for the associated symbol.

No need. This function has nothing to do with the binding in the associated level. If calling this function in the associated level symbol map stack when there is no binding for the associated symbol, you can get the host value. The difference between lookupSymbol and lookupHostSymbol for this case (has not created the binding for the associated symbol) is that lookupSymbol does not find the symbol box in the associated level symbol map stack and lookupHostSymbol skip the associated level symbol map stack.

What I mean here is that if you skip one level, there is a possibility that you can pop out the map containing the entry for the host version.

Sorry, I think I was wrong here. Since we always push before we enter an OpenMP region, i guess this issue i mentioned can never happen.

[kiranchandramohan]

The general point I am trying to make is that it will be good to ensure that the symboxBox we used to create a clone is the same as the one that is used as the source of the copy for Firstprivate. If we can add an assert for this then that will be great.

[peixin]

In D127468#3596810, @kiranchandramohan wrote:

The general point I am trying to make is that it will be good to ensure that the symboxBox we used to create a clone is the same as the one that is used as the source of the copy for Firstprivate. If we can add an assert for this then that will be great.

Yes, it is good to have it if possible. Unfortunately, we cannot have this assertion. If the symboxBox we used to create a clone is known, there is no need for the helper function getting the source of the copy for Firstprivate since we can use the SymbolBox directly. In addition, the symboxBox we used to create a clone is obtained from the loopupSymbol, which means that the binding for the associated symbol does not exist. In this case, loopupSymbol and loopupHostSymbol are totally the same, so there is no need for this assertion. Instead, one assertion is added to check the symbolBox obtained from loopupHostSymbol if something goes wrong.

All in all, binding symbol and looking up symbol always refer to ultimate symbol in different symbol map stack (https://github.com/llvm/llvm-project/blob/44e8a205f4cf747b920726428ee9e35c2ac3d706/flang/include/flang/Lower/SymbolMap.h#L338 , https://github.com/llvm/llvm-project/blob/44e8a205f4cf747b920726428ee9e35c2ac3d706/flang/lib/Lower/SymbolMap.cpp#L35). Therefore, if calling loopupSymbol for host-association case can get the symboxBox for the host symbol, calling loopupHostSymbol can get, too.

[kiranchandramohan]

In D127468#3597706, @peixin wrote:

In D127468#3596810, @kiranchandramohan wrote:

The general point I am trying to make is that it will be good to ensure that the symboxBox we used to create a clone is the same as the one that is used as the source of the copy for Firstprivate. If we can add an assert for this then that will be great.

Yes, it is good to have it if possible. Unfortunately, we cannot have this assertion. If the symboxBox we used to create a clone is known, there is no need for the helper function getting the source of the copy for Firstprivate since we can use the SymbolBox directly. In addition, the symboxBox we used to create a clone is obtained from the loopupSymbol, which means that the binding for the associated symbol does not exist. In this case, loopupSymbol and loopupHostSymbol are totally the same, so there is no need for this assertion. Instead, one assertion is added to check the symbolBox obtained from loopupHostSymbol if something goes wrong.

All in all, binding symbol and looking up symbol always refer to ultimate symbol in different symbol map stack (https://github.com/llvm/llvm-project/blob/44e8a205f4cf747b920726428ee9e35c2ac3d706/flang/include/flang/Lower/SymbolMap.h#L338 , https://github.com/llvm/llvm-project/blob/44e8a205f4cf747b920726428ee9e35c2ac3d706/flang/lib/Lower/SymbolMap.cpp#L35). Therefore, if calling loopupSymbol for host-association case can get the symboxBox for the host symbol, calling loopupHostSymbol can get, too.

OK. Can we add some kind of assert, like the following?
-> lookUpSymbol != lookUpHostSymbol
-> There exists a binding of the symbol while doing a shallow lookup.

[peixin]

Add the assertions.

[peixin]

-> lookUpSymbol != lookUpHostSymbol

Added this kind of check.

-> There exists a binding of the symbol while doing a shallow lookup.

Good point, shallowLookupSymbol is more strict. Fixed.

[kiranchandramohan]

LGTM.

Have a few comments about renaming.

flang/include/flang/Lower/SymbolMap.h

307–308

Nit: would nonShallowLookupSymbol or nonLocalLookupSymbol be a better name? You can explain in the comment that this is typically used in OpenMP code for getting host variable bindings.

flang/lib/Lower/OpenMP.cpp

207

Nit: no auto.

209

Nit: use = in Lowering code.

flang/lib/Lower/SymbolMap.cpp

55

Nit: Would lookOneLevelUpSymbol be a better name? You can add a comment saying that this is generally used to get mappings of host symbols.

[peixin]

Thanks @kiranchandramohan for the suggestions. loopupOneLevelUpSymbol is better and comments itself. Address the comments.

[peixin]

Rebase before land.

flang/include/flang/Lower/AbstractConverter.h

public:
   createHostAssociateVarClone(const Fortran::semantics::Symbol &sym) = 0;
 
   virtual void copyHostAssociateVar(const Fortran::semantics::Symbol &sym) = 0;
 
   /// Collect the set of symbols flagged as \p flag in \p eval region.
   virtual void collectSymbolSet(
       pft::Evaluation &eval,
       llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet,
-      Fortran::semantics::Symbol::Flag flag) = 0;
+      Fortran::semantics::Symbol::Flag flag, bool isOriginalSymbol = false) = 0;
 
   //===--------------------------------------------------------------------===//
   // Expressions
   //===--------------------------------------------------------------------===//
 
   /// Generate the address of the location holding the expression, \p expr.
   /// If \p expr is a Designator that is not compile time contiguous, the
   /// address returned is the one of a contiguous temporary storage holding the

flang/include/flang/Lower/SymbolMap.h

public:
 
   /// Find `symbol` and return its value if it appears in the inner-most level
   /// map.
   SymbolBox shallowLookupSymbol(semantics::SymbolRef sym);
   SymbolBox shallowLookupSymbol(const semantics::Symbol *sym) {
     return shallowLookupSymbol(*sym);
   }
 
+  /// Find `symbol` and return its value if it appears in the host level map
+  /// for the host variable in host-association.
+  SymbolBox lookupHostSymbol(semantics::SymbolRef sym);
+  SymbolBox lookupHostSymbol(const semantics::Symbol *sym) {
+    return lookupHostSymbol(*sym);
+  }
+
   /// Add a new binding from the ac-do-variable `var` to `value`.
   void pushImpliedDoBinding(AcDoVar var, mlir::Value value) {
     impliedDoStack.emplace_back(var, value);
   }
 
   /// Pop the most recent implied do binding off the stack.
   void popImpliedDoBinding() {
     assert(!impliedDoStack.empty());

flang/lib/Lower/Bridge.cpp

public:
   }
 
   void
   copyHostAssociateVar(const Fortran::semantics::Symbol &sym) override final {
     // 1) Fetch the original copy of the variable.
     assert(sym.has<Fortran::semantics::HostAssocDetails>() &&
            "No host-association found");
     const Fortran::semantics::Symbol &hsym = sym.GetUltimate();
-    Fortran::lower::SymbolBox hsb = lookupSymbol(hsym);
+    Fortran::lower::SymbolBox hsb = lookupHostSymbol(hsym);
+    assert(hsb && "Host symbol box not found");
     fir::ExtendedValue hexv = getExtendedValue(hsb);
 
-    // 2) Create a copy that will mask the original.
-    createHostAssociateVarClone(sym);
+    // 2) Fetch the copied one that will mask the original.
     Fortran::lower::SymbolBox sb = lookupSymbol(sym);
+    assert(sb && "Host-associated symbol box not found");
     fir::ExtendedValue exv = getExtendedValue(sb);
 
     // 3) Perform the assignment.
-    mlir::Location loc = genLocation(sym.name());
+    builder->setInsertionPointAfter(fir::getBase(exv).getDefiningOp());
+    mlir::Location loc = getCurrentLocation();
     mlir::Type symType = genType(sym);
     if (auto seqTy = symType.dyn_cast<fir::SequenceType>()) {
       Fortran::lower::StatementContext stmtCtx;
       Fortran::lower::createSomeArrayAssignment(*this, exv, hexv, localSymbols,
                                                 stmtCtx);
       stmtCtx.finalize();
     } else if (hexv.getBoxOf<fir::CharBoxValue>()) {
       fir::factory::CharacterExprHelper{*builder, loc}.createAssign(exv, hexv);
     } else if (hexv.getBoxOf<fir::MutableBoxValue>()) {
       TODO(loc, "firstprivatisation of allocatable variables");
     } else {
       auto loadVal = builder->create<fir::LoadOp>(loc, fir::getBase(hexv));
       builder->create<fir::StoreOp>(loc, loadVal, fir::getBase(exv));
     }
   }
 
   //===--------------------------------------------------------------------===//
   // Utility methods
   //===--------------------------------------------------------------------===//
 
   void collectSymbolSet(
       Fortran::lower::pft::Evaluation &eval,
       llvm::SetVector<const Fortran::semantics::Symbol *> &symbolSet,
-      Fortran::semantics::Symbol::Flag flag) override final {
+      Fortran::semantics::Symbol::Flag flag,
+      bool isOriginalSymbol) override final {
     auto addToList = [&](const Fortran::semantics::Symbol &sym) {
-      const Fortran::semantics::Symbol &ultimate = sym.GetUltimate();
-      if (ultimate.test(flag))
-        symbolSet.insert(&ultimate);
+      const Fortran::semantics::Symbol &symbol =
+          isOriginalSymbol ? sym : sym.GetUltimate();
+      if (symbol.test(flag))
+        symbolSet.insert(&symbol);
     };
     Fortran::lower::pft::visitAllSymbols(eval, addToList);
   }
 
   mlir::Location getCurrentLocation() override final { return toLocation(); }
 
   /// Generate a dummy location.
   mlir::Location genUnknownLocation() override final {
private:
   /// Find the symbol in the inner-most level of the local map or return null.
   Fortran::lower::SymbolBox
   shallowLookupSymbol(const Fortran::semantics::Symbol &sym) {
     if (Fortran::lower::SymbolBox v = localSymbols.shallowLookupSymbol(sym))
       return v;
     return {};
   }
 
+  /// Find the symbol in the host level of symbol map for host-association
+  /// or return null.
+  Fortran::lower::SymbolBox
+  lookupHostSymbol(const Fortran::semantics::Symbol &sym) {
+    if (Fortran::lower::SymbolBox v = localSymbols.lookupHostSymbol(sym))
+      return v;
+    return {};
+  }
+
   /// Add the symbol to the local map and return `true`. If the symbol is
   /// already in the map and \p forced is `false`, the map is not updated.
   /// Instead the value `false` is returned.
   bool addSymbol(const Fortran::semantics::SymbolRef sym, mlir::Value val,
                  bool forced = false) {
     if (!forced && lookupSymbol(sym))
       return false;
     localSymbols.addSymbol(sym, val, forced);

flang/lib/Lower/OpenMP.cpp

static void createPrivateVarSyms(Fortran::lower::AbstractConverter &converter,
                                  const T *clause) {
   const Fortran::parser::OmpObjectList &ompObjectList = clause->v;
   for (const Fortran::parser::OmpObject &ompObject : ompObjectList.v) {
     Fortran::semantics::Symbol *sym = getOmpObjectSymbol(ompObject);
     // Privatization for symbols which are pre-determined (like loop index
     // variables) happen separately, for everything else privatize here.
     if (sym->test(Fortran::semantics::Symbol::Flag::OmpPreDetermined))
       continue;
-    if constexpr (std::is_same_v<T, Fortran::parser::OmpClause::Firstprivate>) {
-      converter.copyHostAssociateVar(*sym);
-    } else {
-      bool success = converter.createHostAssociateVarClone(*sym);
-      (void)success;
-      assert(success && "Privatization failed due to existing binding");
+    if constexpr (std::is_same_v<T, Fortran::parser::OmpClause::Firstprivate>) {
+      converter.copyHostAssociateVar(*sym);
     }
   }
 }
 
 static void privatizeVars(Fortran::lower::AbstractConverter &converter,
                           const Fortran::parser::OmpClauseList &opClauseList) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   auto insPt = firOpBuilder.saveInsertionPoint();
assert(mlir::isa<mlir::omp::ThreadprivateOp>(op) &&
            "The threadprivate operation not created");
     mlir::Value symValue =
         mlir::dyn_cast<mlir::omp::ThreadprivateOp>(op).sym_addr();
     return firOpBuilder.create<mlir::omp::ThreadprivateOp>(
         currentLocation, symValue.getType(), symValue);
   };
 
   llvm::SetVector<const Fortran::semantics::Symbol *> threadprivateSyms;
-  converter.collectSymbolSet(
-      eval, threadprivateSyms,
-      Fortran::semantics::Symbol::Flag::OmpThreadprivate);
+  converter.collectSymbolSet(eval, threadprivateSyms,
+                             Fortran::semantics::Symbol::Flag::OmpThreadprivate,
+                             /*isOriginalSymbol=*/true);
+  std::set<Fortran::semantics::SourceName> threadprivateSymNames;
 
   // For a COMMON block, the ThreadprivateOp is generated for itself instead of
   // its members, so only bind the value of the new copied ThreadprivateOp
   // inside the parallel region to the common block symbol only once for
   // multiple members in one COMMON block.
   llvm::SetVector<const Fortran::semantics::Symbol *> commonSyms;
   for (std::size_t i = 0; i < threadprivateSyms.size(); i++) {
     auto sym = threadprivateSyms[i];
     mlir::Value symThreadprivateValue;
+    // The variable may be used more than once, and each reference has one
+    // symbol with the same name. Only do once for references of one variable.
+    if (threadprivateSymNames.find(sym->name()) != threadprivateSymNames.end())
+      continue;
+    else
+      threadprivateSymNames.insert(sym->name());
     if (const Fortran::semantics::Symbol *common =
             Fortran::semantics::FindCommonBlockContaining(sym->GetUltimate())) {
       mlir::Value commonThreadprivateValue;
       if (commonSyms.contains(common)) {
         commonThreadprivateValue = converter.getSymbolAddress(*common);
       } else {
         commonThreadprivateValue = genThreadprivateOp(*common);
         converter.bindSymbol(*common, commonThreadprivateValue);
for (std::size_t i = 0; i < threadprivateSyms.size(); i++) {
     fir::ExtendedValue symThreadprivateExv =
         getExtendedValue(sexv, symThreadprivateValue);
     converter.bindSymbol(*sym, symThreadprivateExv);
   }
 
   firOpBuilder.restoreInsertionPoint(insPt);
 }
 
+static void
+genCopyinClause(Fortran::lower::AbstractConverter &converter,
+                const Fortran::parser::OmpClauseList &opClauseList) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  auto insPt = firOpBuilder.saveInsertionPoint();
+  firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
+  bool hasCopyin{false};
+  for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
+    if (const auto &copyinClause =
+            std::get_if<Fortran::parser::OmpClause::Copyin>(&clause.u)) {
+      hasCopyin = true;
+      const Fortran::parser::OmpObjectList &ompObjectList = copyinClause->v;
+      for (const Fortran::parser::OmpObject &ompObject : ompObjectList.v) {
+        Fortran::semantics::Symbol *sym = getOmpObjectSymbol(ompObject);
+        if (sym->has<Fortran::semantics::CommonBlockDetails>())
+          TODO(converter.getCurrentLocation(), "common block in Copyin clause");
+        if (Fortran::semantics::IsAllocatableOrPointer(sym->GetUltimate()))
+          TODO(converter.getCurrentLocation(),
+               "pointer or allocatable variables in Copyin clause");
+        assert(sym->has<Fortran::semantics::HostAssocDetails>() &&
+               "No host-association found");
+        converter.copyHostAssociateVar(*sym);
+      }
+    }
+  }
+  if (hasCopyin)
+    firOpBuilder.create<mlir::omp::BarrierOp>(converter.getCurrentLocation());
+  firOpBuilder.restoreInsertionPoint(insPt);
+}
+
 static void genObjectList(const Fortran::parser::OmpObjectList &objectList,
                           Fortran::lower::AbstractConverter &converter,
                           llvm::SmallVectorImpl<Value> &operands) {
   auto addOperands = [&](Fortran::lower::SymbolRef sym) {
     const mlir::Value variable = converter.getSymbolAddress(sym);
     if (variable) {
       operands.push_back(variable);
     } else {
if (storeOp)
     firOpBuilder.setInsertionPointAfter(storeOp);
   else
     firOpBuilder.setInsertionPointToStart(&block);
 
   // Handle privatization. Do not privatize if this is the outer operation.
   if (clauses && !outerCombined)
     privatizeVars(converter, *clauses);
 
-  if (std::is_same_v<Op, omp::ParallelOp>)
+  if (std::is_same_v<Op, omp::ParallelOp>) {
     threadPrivatizeVars(converter, eval);
+    if (clauses)
+      genCopyinClause(converter, *clauses);
+  }
 }
 
 static void genOMP(Fortran::lower::AbstractConverter &converter,
                    Fortran::lower::pft::Evaluation &eval,
                    const Fortran::parser::OpenMPSimpleStandaloneConstruct
                        &simpleStandaloneConstruct) {
   const auto &directive =
       std::get<Fortran::parser::OmpSimpleStandaloneDirective>(
createCombinedParallelOp(Fortran::lower::AbstractConverter &converter,
   llvm::ArrayRef<mlir::Type> argTy;
   mlir::Value ifClauseOperand, numThreadsClauseOperand;
   SmallVector<Value> allocatorOperands, allocateOperands;
   mlir::omp::ClauseProcBindKindAttr procBindKindAttr;
   const auto &opClauseList =
       std::get<Fortran::parser::OmpClauseList>(directive.t);
   // TODO: Handle the following clauses
   // 1. default
-  // 2. copyin
   // Note: rest of the clauses are handled when the inner operation is created
   for (const Fortran::parser::OmpClause &clause : opClauseList.v) {
     if (const auto &ifClause =
             std::get_if<Fortran::parser::OmpClause::If>(&clause.u)) {
       auto &expr = std::get<Fortran::parser::ScalarLogicalExpr>(ifClause->v.t);
       mlir::Value ifVal = fir::getBase(
           converter.genExprValue(*Fortran::semantics::GetExpr(expr), stmtCtx));
       ifClauseOperand = firOpBuilder.createConvert(
if (const auto &ifClause =
       procBindKindAttr = genProcBindKindAttr(firOpBuilder, procBindClause);
     } else if (const auto &allocateClause =
                    std::get_if<Fortran::parser::OmpClause::Allocate>(
                        &clause.u)) {
       genAllocateClause(converter, allocateClause->v, allocatorOperands,
                         allocateOperands);
     } else if (std::get_if<Fortran::parser::OmpClause::Private>(&clause.u) ||
                std::get_if<Fortran::parser::OmpClause::Firstprivate>(
-                   &clause.u)) {
-      // Privatisation clauses are handled elsewhere.
+                   &clause.u) ||
+               std::get_if<Fortran::parser::OmpClause::Copyin>(&clause.u)) {
+      // Privatisation and copyin clauses are handled elsewhere.
       continue;
     } else if (std::get_if<Fortran::parser::OmpClause::Threads>(&clause.u)) {
       // Nothing needs to be done for threads clause.
       continue;
     } else if (const auto &finalClause =
                    std::get_if<Fortran::parser::OmpClause::Final>(&clause.u)) {
       mlir::Value finalVal = fir::getBase(converter.genExprValue(
           *Fortran::semantics::GetExpr(finalClause->v), stmtCtx));

flang/lib/Lower/SymbolMap.cpp

Fortran::lower::SymbolBox Fortran::lower::SymMap::shallowLookupSymbol(
     Fortran::semantics::SymbolRef symRef) {
   auto &map = symbolMapStack.back();
   auto iter = map.find(&symRef.get().GetUltimate());
   if (iter != map.end())
     return iter->second;
   return SymbolBox::None{};
 }
 
+Fortran::lower::SymbolBox
+Fortran::lower::SymMap::lookupHostSymbol(Fortran::semantics::SymbolRef symRef) {
+  Fortran::semantics::SymbolRef sym = symRef.get().GetUltimate();
+  auto jmap = symbolMapStack.rbegin();
+  auto jend = symbolMapStack.rend();
+  if (jmap == jend)
+    return SymbolBox::None{};
+  // Skip the associated SymMap stack level.
+  for (++jmap; jmap != jend; ++jmap) {
+    auto iter = jmap->find(&*sym);
+    if (iter != jmap->end())
+      return iter->second;
+  }
+  return SymbolBox::None{};
+}
+
 mlir::Value
 Fortran::lower::SymMap::lookupImpliedDo(Fortran::lower::SymMap::AcDoVar var) {
   for (auto [marker, binding] : llvm::reverse(impliedDoStack))
     if (var == marker)
       return binding;
   return {};
 }
 

flang/lib/Semantics/resolve-directives.cpp

private:
 
   static constexpr Symbol::Flags privateDataSharingAttributeFlags{
       Symbol::Flag::OmpPrivate, Symbol::Flag::OmpFirstPrivate,
       Symbol::Flag::OmpLastPrivate};
 
   static constexpr Symbol::Flags ompFlagsRequireNewSymbol{
       Symbol::Flag::OmpPrivate, Symbol::Flag::OmpLinear,
       Symbol::Flag::OmpFirstPrivate, Symbol::Flag::OmpLastPrivate,
-      Symbol::Flag::OmpReduction, Symbol::Flag::OmpCriticalLock};
+      Symbol::Flag::OmpReduction, Symbol::Flag::OmpCriticalLock,
+      Symbol::Flag::OmpCopyIn};
 
   static constexpr Symbol::Flags ompFlagsRequireMark{
       Symbol::Flag::OmpThreadprivate};
 
   static constexpr Symbol::Flags dataCopyingAttributeFlags{
       Symbol::Flag::OmpCopyIn, Symbol::Flag::OmpCopyPrivate};
 
   std::vector<const parser::Name *> allocateNames_; // on one directive

 }
 
 template <typename T>
 Symbol *DirectiveAttributeVisitor<T>::DeclarePrivateAccessEntity(
     Symbol &object, Symbol::Flag flag, Scope &scope) {
   if (object.owner() != currScope()) {
     auto &symbol{MakeAssocSymbol(object.name(), object, scope)};
     symbol.set(flag);
+    if (flag == Symbol::Flag::OmpCopyIn) {
+      // The symbol in copyin clause must be threadprivate entity.
+      symbol.set(Symbol::Flag::OmpThreadprivate);
+    }
     return &symbol;
   } else {
     object.set(flag);
     return &object;
   }
 }
 
 bool AccAttributeVisitor::Pre(const parser::OpenACCBlockConstruct &x) {

flang/test/Lower/OpenMP/copyin.f90

+! This test checks lowering of `COPYIN` clause.
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck %s
+! RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
+
+! CHECK-LABEL: func.func @_QPcopyin_scalar_array() {
+! CHECK:         %[[VAL_0:.*]] = fir.address_of(@_QFcopyin_scalar_arrayEx1) : !fir.ref<i32>
+! CHECK:         %[[VAL_1:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:         %[[VAL_2:.*]] = fir.address_of(@_QFcopyin_scalar_arrayEx2) : !fir.ref<!fir.array<10xi64>>
+! CHECK:         %[[VAL_3:.*]] = arith.constant 10 : index
+! CHECK:         %[[VAL_4:.*]] = omp.threadprivate %[[VAL_2]] : !fir.ref<!fir.array<10xi64>> -> !fir.ref<!fir.array<10xi64>>
+! CHECK:         omp.parallel   {
+! CHECK:           %[[VAL_5:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:           %[[VAL_6:.*]] = fir.load %[[VAL_1]] : !fir.ref<i32>
+! CHECK:           fir.store %[[VAL_6]] to %[[VAL_5]] : !fir.ref<i32>
+! CHECK:           %[[VAL_7:.*]] = omp.threadprivate %[[VAL_2]] : !fir.ref<!fir.array<10xi64>> -> !fir.ref<!fir.array<10xi64>>
+! CHECK:           %[[VAL_8:.*]] = fir.shape %[[VAL_3]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_9:.*]] = fir.array_load %[[VAL_7]](%[[VAL_8]]) : (!fir.ref<!fir.array<10xi64>>, !fir.shape<1>) -> !fir.array<10xi64>
+! CHECK:           %[[VAL_10:.*]] = fir.shape %[[VAL_3]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_11:.*]] = fir.array_load %[[VAL_4]](%[[VAL_10]]) : (!fir.ref<!fir.array<10xi64>>, !fir.shape<1>) -> !fir.array<10xi64>
+! CHECK:           %[[VAL_12:.*]] = arith.constant 1 : index
+! CHECK:           %[[VAL_13:.*]] = arith.constant 0 : index
+! CHECK:           %[[VAL_14:.*]] = arith.subi %[[VAL_3]], %[[VAL_12]] : index
+! CHECK:           %[[VAL_15:.*]] = fir.do_loop %[[VAL_16:.*]] = %[[VAL_13]] to %[[VAL_14]] step %[[VAL_12]] unordered iter_args(%[[VAL_17:.*]] = %[[VAL_9]]) -> (!fir.array<10xi64>) {
+! CHECK:             %[[VAL_18:.*]] = fir.array_fetch %[[VAL_11]], %[[VAL_16]] : (!fir.array<10xi64>, index) -> i64
+! CHECK:             %[[VAL_19:.*]] = fir.array_update %[[VAL_17]], %[[VAL_18]], %[[VAL_16]] : (!fir.array<10xi64>, i64, index) -> !fir.array<10xi64>
+! CHECK:             fir.result %[[VAL_19]] : !fir.array<10xi64>
+! CHECK:           }
+! CHECK:           fir.array_merge_store %[[VAL_9]], %[[VAL_20:.*]] to %[[VAL_7]] : !fir.array<10xi64>, !fir.array<10xi64>, !fir.ref<!fir.array<10xi64>>
+! CHECK:           omp.barrier
+! CHECK:           fir.call @_QPsub1(%[[VAL_5]], %[[VAL_7]]) : (!fir.ref<i32>, !fir.ref<!fir.array<10xi64>>) -> ()
+! CHECK:           omp.terminator
+! CHECK:         }
+! CHECK:         return
+! CHECK:       }
+
+subroutine copyin_scalar_array()
+  integer(kind=4), save :: x1
+  integer(kind=8), save :: x2(10)
+  !$omp threadprivate(x1, x2)
+
+  !$omp parallel copyin(x1) copyin(x2)
+    call sub1(x1, x2)
+  !$omp end parallel
+
+end
+
+! CHECK-LABEL: func.func @_QPcopyin_char_chararray() {
+! CHECK:         %[[VAL_0:.*]] = fir.address_of(@_QFcopyin_char_chararrayEx3) : !fir.ref<!fir.char<1,5>>
+! CHECK:         %[[VAL_1:.*]] = arith.constant 5 : index
+! CHECK:         %[[VAL_2:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<!fir.char<1,5>> -> !fir.ref<!fir.char<1,5>>
+! CHECK:         %[[VAL_3:.*]] = fir.address_of(@_QFcopyin_char_chararrayEx4) : !fir.ref<!fir.array<10x!fir.char<1,5>>>
+! CHECK:         %[[VAL_4:.*]] = arith.constant 5 : index
+! CHECK:         %[[VAL_5:.*]] = arith.constant 10 : index
+! CHECK:         %[[VAL_6:.*]] = omp.threadprivate %[[VAL_3]] : !fir.ref<!fir.array<10x!fir.char<1,5>>> -> !fir.ref<!fir.array<10x!fir.char<1,5>>>
+! CHECK:         omp.parallel   {
+! CHECK:           %[[VAL_7:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<!fir.char<1,5>> -> !fir.ref<!fir.char<1,5>>
+! CHECK:           %[[VAL_8:.*]] = arith.constant 1 : i64
+! CHECK:           %[[VAL_9:.*]] = fir.convert %[[VAL_1]] : (index) -> i64
+! CHECK:           %[[VAL_10:.*]] = arith.muli %[[VAL_8]], %[[VAL_9]] : i64
+! CHECK:           %[[VAL_11:.*]] = arith.constant false
+! CHECK:           %[[VAL_12:.*]] = fir.convert %[[VAL_7]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
+! CHECK:           %[[VAL_13:.*]] = fir.convert %[[VAL_2]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
+! CHECK:           fir.call @llvm.memmove.p0.p0.i64(%[[VAL_12]], %[[VAL_13]], %[[VAL_10]], %[[VAL_11]]) : (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+! CHECK:           %[[VAL_14:.*]] = omp.threadprivate %[[VAL_3]] : !fir.ref<!fir.array<10x!fir.char<1,5>>> -> !fir.ref<!fir.array<10x!fir.char<1,5>>>
+! CHECK:           %[[VAL_15:.*]] = fir.shape %[[VAL_5]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_16:.*]] = fir.array_load %[[VAL_14]](%[[VAL_15]]) : (!fir.ref<!fir.array<10x!fir.char<1,5>>>, !fir.shape<1>) -> !fir.array<10x!fir.char<1,5>>
+! CHECK:           %[[VAL_17:.*]] = fir.shape %[[VAL_5]] : (index) -> !fir.shape<1>
+! CHECK:           %[[VAL_18:.*]] = fir.array_load %[[VAL_6]](%[[VAL_17]]) : (!fir.ref<!fir.array<10x!fir.char<1,5>>>, !fir.shape<1>) -> !fir.array<10x!fir.char<1,5>>
+! CHECK:           %[[VAL_19:.*]] = arith.constant 1 : index
+! CHECK:           %[[VAL_20:.*]] = arith.constant 0 : index
+! CHECK:           %[[VAL_21:.*]] = arith.subi %[[VAL_5]], %[[VAL_19]] : index
+! CHECK:           %[[VAL_22:.*]] = fir.do_loop %[[VAL_23:.*]] = %[[VAL_20]] to %[[VAL_21]] step %[[VAL_19]] unordered iter_args(%[[VAL_24:.*]] = %[[VAL_16]]) -> (!fir.array<10x!fir.char<1,5>>) {
+! CHECK:             %[[VAL_25:.*]] = fir.array_access %[[VAL_18]], %[[VAL_23]] : (!fir.array<10x!fir.char<1,5>>, index) -> !fir.ref<!fir.char<1,5>>
+! CHECK:             %[[VAL_26:.*]] = fir.array_access %[[VAL_24]], %[[VAL_23]] : (!fir.array<10x!fir.char<1,5>>, index) -> !fir.ref<!fir.char<1,5>>
+! CHECK:             %[[VAL_27:.*]] = arith.constant 5 : index
+! CHECK:             %[[VAL_28:.*]] = arith.constant 1 : i64
+! CHECK:             %[[VAL_29:.*]] = fir.convert %[[VAL_27]] : (index) -> i64
+! CHECK:             %[[VAL_30:.*]] = arith.muli %[[VAL_28]], %[[VAL_29]] : i64
+! CHECK:             %[[VAL_31:.*]] = arith.constant false
+! CHECK:             %[[VAL_32:.*]] = fir.convert %[[VAL_26]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
+! CHECK:             %[[VAL_33:.*]] = fir.convert %[[VAL_25]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<i8>
+! CHECK:             fir.call @llvm.memmove.p0.p0.i64(%[[VAL_32]], %[[VAL_33]], %[[VAL_30]], %[[VAL_31]]) : (!fir.ref<i8>, !fir.ref<i8>, i64, i1) -> ()
+! CHECK:             %[[VAL_34:.*]] = fir.array_amend %[[VAL_24]], %[[VAL_26]] : (!fir.array<10x!fir.char<1,5>>, !fir.ref<!fir.char<1,5>>) -> !fir.array<10x!fir.char<1,5>>
+! CHECK:             fir.result %[[VAL_34]] : !fir.array<10x!fir.char<1,5>>
+! CHECK:           }
+! CHECK:           fir.array_merge_store %[[VAL_16]], %[[VAL_35:.*]] to %[[VAL_14]] : !fir.array<10x!fir.char<1,5>>, !fir.array<10x!fir.char<1,5>>, !fir.ref<!fir.array<10x!fir.char<1,5>>>
+! CHECK:           omp.barrier
+! CHECK:           %[[VAL_36:.*]] = fir.convert %[[VAL_7]] : (!fir.ref<!fir.char<1,5>>) -> !fir.ref<!fir.char<1,?>>
+! CHECK:           %[[VAL_37:.*]] = fir.emboxchar %[[VAL_36]], %[[VAL_1]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:           %[[VAL_38:.*]] = fir.convert %[[VAL_14]] : (!fir.ref<!fir.array<10x!fir.char<1,5>>>) -> !fir.ref<!fir.char<1,?>>
+! CHECK:           %[[VAL_39:.*]] = fir.emboxchar %[[VAL_38]], %[[VAL_4]] : (!fir.ref<!fir.char<1,?>>, index) -> !fir.boxchar<1>
+! CHECK:           fir.call @_QPsub2(%[[VAL_37]], %[[VAL_39]]) : (!fir.boxchar<1>, !fir.boxchar<1>) -> ()
+! CHECK:           omp.terminator
+! CHECK:         }
+! CHECK:         return
+! CHECK:       }
+
+subroutine copyin_char_chararray()
+  character(5), save :: x3, x4(10)
+  !$omp threadprivate(x3, x4)
+
+  !$omp parallel copyin(x3) copyin(x4)
+    call sub2(x3, x4)
+  !$omp end parallel
+
+end
+
+! CHECK-LABEL: func.func @_QPcopyin_derived_type() {
+! CHECK:         %[[VAL_0:.*]] = fir.address_of(@_QFcopyin_derived_typeEx5) : !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>
+! CHECK:         %[[VAL_1:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>> -> !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>
+! CHECK:         omp.parallel   {
+! CHECK:           %[[VAL_2:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>> -> !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>
+! CHECK:           %[[VAL_3:.*]] = fir.load %[[VAL_1]] : !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>
+! CHECK:           fir.store %[[VAL_3]] to %[[VAL_2]] : !fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>
+! CHECK:           omp.barrier
+! CHECK:           fir.call @_QPsub3(%[[VAL_2]]) : (!fir.ref<!fir.type<_QFcopyin_derived_typeTmy_type{t_i:i32,t_arr:!fir.array<5xi32>}>>) -> ()
+! CHECK:           omp.terminator
+! CHECK:         }
+! CHECK:         return
+! CHECK:       }
+
+subroutine copyin_derived_type()
+  type my_type
+    integer :: t_i
+    integer :: t_arr(5)
+  end type my_type
+  type(my_type), save :: x5
+  !$omp threadprivate(x5)
+
+  !$omp parallel copyin(x5)
+    call sub3(x5)
+  !$omp end parallel
+
+end
+
+! CHECK-LABEL: func.func @_QPcombined_parallel_worksharing_loop() {
+! CHECK:         %[[VAL_0:.*]] = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFcombined_parallel_worksharing_loopEi"}
+! CHECK:         %[[VAL_1:.*]] = fir.address_of(@_QFcombined_parallel_worksharing_loopEx6) : !fir.ref<i32>
+! CHECK:         %[[VAL_2:.*]] = omp.threadprivate %[[VAL_1]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:         omp.parallel   {
+! CHECK:           %[[VAL_3:.*]] = fir.alloca i32 {adapt.valuebyref, pinned}
+! CHECK:           %[[VAL_4:.*]] = omp.threadprivate %[[VAL_1]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:           %[[VAL_5:.*]] = fir.load %[[VAL_2]] : !fir.ref<i32>
+! CHECK:           fir.store %[[VAL_5]] to %[[VAL_4]] : !fir.ref<i32>
+! CHECK:           omp.barrier
+! CHECK:           %[[VAL_6:.*]] = arith.constant 1 : i32
+! CHECK:           %[[VAL_7:.*]] = fir.load %[[VAL_4]] : !fir.ref<i32>
+! CHECK:           %[[VAL_8:.*]] = arith.constant 1 : i32
+! CHECK:           omp.wsloop   for  (%[[VAL_9:.*]]) : i32 = (%[[VAL_6]]) to (%[[VAL_7]]) inclusive step (%[[VAL_8]]) {
+! CHECK:             fir.store %[[VAL_9]] to %[[VAL_3]] : !fir.ref<i32>
+! CHECK:             fir.call @_QPsub4(%[[VAL_4]]) : (!fir.ref<i32>) -> ()
+! CHECK:             omp.yield
+! CHECK:           }
+! CHECK:           omp.terminator
+! CHECK:         }
+! CHECK:         return
+! CHECK:       }
+
+subroutine combined_parallel_worksharing_loop()
+  integer, save :: x6
+  !$omp threadprivate(x6)
+
+  !$omp parallel do copyin(x6)
+    do i=1, x6
+      call sub4(x6)
+    end do
+  !$omp end parallel do
+
+end
+
+! CHECK-LABEL: func.func @_QPcombined_parallel_sections() {
+! CHECK:         %[[VAL_0:.*]] = fir.address_of(@_QFcombined_parallel_sectionsEx7) : !fir.ref<i32>
+! CHECK:         %[[VAL_1:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:         omp.parallel   {
+! CHECK:           %[[VAL_2:.*]] = omp.threadprivate %[[VAL_0]] : !fir.ref<i32> -> !fir.ref<i32>
+! CHECK:           %[[VAL_3:.*]] = fir.load %[[VAL_1]] : !fir.ref<i32>
+! CHECK:           fir.store %[[VAL_3]] to %[[VAL_2]] : !fir.ref<i32>
+! CHECK:           omp.barrier
+! CHECK:           omp.sections   {
+! CHECK:             omp.section {
+! CHECK:               fir.call @_QPsub5(%[[VAL_2]]) : (!fir.ref<i32>) -> ()
+! CHECK:               omp.terminator
+! CHECK:             }
+! CHECK:             omp.section {
+! CHECK:               fir.call @_QPsub6(%[[VAL_2]]) : (!fir.ref<i32>) -> ()
+! CHECK:               omp.terminator
+! CHECK:             }
+! CHECK:             omp.terminator
+! CHECK:           }
+! CHECK:           omp.terminator
+! CHECK:         }
+! CHECK:         return
+! CHECK:       }
+
+subroutine combined_parallel_sections()
+  integer, save :: x7
+  !$omp threadprivate(x7)
+
+  !$omp parallel sections copyin(x7)
+    !$omp section
+      call sub5(x7)
+    !$omp section
+      call sub6(x7)
+  !$omp end parallel sections
+
+end