Skip to content

[flang] Lowering changes for assigning dummy_scope to hlfir.declare. #90989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 8, 2024

Conversation

vzakhari
Copy link
Contributor

@vzakhari vzakhari commented May 3, 2024

The lowering produces fir.dummy_scope operation if the current
function has dummy arguments. Each hlfir.declare generated
for a dummy argument is then using the result of fir.dummy_scope
as its dummy_scope operand. This is only done for HLFIR.

I was not able to find a reliable way to identify dummy symbols
in genDeclareSymbol, so I added a set of registered dummy symbols
that is alive during the variables instantiation for the current
function. The set is initialized during the mapping of the dummy
argument symbols to their MLIR values. It is reset right after
all variables are instantiated - this is done to avoid generating
hlfir.declare operations with dummy_scope for the clones of
the dummy symbols (e.g. this happens with OpenMP privatization).

If this can be done in a cleaner way, please advise.

@vzakhari vzakhari requested review from tblah and jeanPerier May 3, 2024 17:57
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I can't think of a cleaner way to do this off the top of my head. You could fix all of the OpenMP code, but it is very likely that more will be added so I think the way you have done it is a better option.

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks you Slava! You approach seems safer. Without more context in genDeclareSymbol, you cannot make a difference between internal procedure dummy vs host procedure dummy symbols for instance.

Good job updating all those tests, did you manage scripting it?


/// Record the given symbol as a dummy argument of this function.
void registerDummySymbol(Fortran::semantics::SymbolRef symRef) {
auto *sym = symRef->HasLocalLocality() ? &*symRef : &symRef->GetUltimate();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there cases where a dummy symbol is not already the ultimate one at the beginning of the procedure scope?
Do you have an example where this line makes a difference compared to *sym = &*symRef. It does not seems incorrect, I just want to understand the rational.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, I do not have an example for this. I was just following the lookupSymbol impl. I will remove it here and in isRegisteredDummySymbol. Thanks!

@vzakhari
Copy link
Contributor Author

vzakhari commented May 6, 2024

Thanks you Slava! You approach seems safer. Without more context in genDeclareSymbol, you cannot make a difference between internal procedure dummy vs host procedure dummy symbols for instance.

Good job updating all those tests, did you manage scripting it?

I did not find a reliable way to script this, so I added dummy_scope ... to the declare ops, and then used this as an "anchor" to add !fir.dscope into the type signature of the operation.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp openacc labels May 6, 2024
@llvmbot
Copy link
Member

llvmbot commented May 6, 2024

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Slava Zakharin (vzakhari)

Changes

The lowering produces fir.dummy_scope operation if the current
function has dummy arguments. Each hlfir.declare generated
for a dummy argument is then using the result of fir.dummy_scope
as its dummy_scope operand. This is only done for HLFIR.

I was not able to find a reliable way to identify dummy symbols
in genDeclareSymbol, so I added a set of registered dummy symbols
that is alive during the variables instantiation for the current
function. The set is initialized during the mapping of the dummy
argument symbols to their MLIR values. It is reset right after
all variables are instantiated - this is done to avoid generating
hlfir.declare operations with dummy_scope for the clones of
the dummy symbols (e.g. this happens with OpenMP privatization).

If this can be done in a cleaner way, please advise.


Patch is 585.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/90989.diff

137 Files Affected:

  • (modified) flang/include/flang/Lower/AbstractConverter.h (+12)
  • (modified) flang/include/flang/Optimizer/Builder/HLFIRTools.h (+1)
  • (modified) flang/include/flang/Optimizer/HLFIR/HLFIROps.td (+1)
  • (modified) flang/lib/Lower/Bridge.cpp (+62-11)
  • (modified) flang/lib/Lower/ConvertArrayConstructor.cpp (+1-1)
  • (modified) flang/lib/Lower/ConvertExprToHLFIR.cpp (+2-1)
  • (modified) flang/lib/Lower/ConvertVariable.cpp (+15-5)
  • (modified) flang/lib/Lower/OpenACC.cpp (+14-8)
  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+6-6)
  • (modified) flang/lib/Optimizer/Builder/HLFIRTools.cpp (+11-8)
  • (modified) flang/lib/Optimizer/Builder/TemporaryStorage.cpp (+2-1)
  • (modified) flang/lib/Optimizer/HLFIR/IR/HLFIROps.cpp (+2-2)
  • (modified) flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp (+8-7)
  • (modified) flang/test/Fir/dispatch.f90 (+1-1)
  • (modified) flang/test/HLFIR/assumed-type-actual-args.f90 (+20-10)
  • (modified) flang/test/HLFIR/assumed_shape_with_value_keyword.f90 (+10-10)
  • (modified) flang/test/HLFIR/boxchar_emboxing.f90 (+2-2)
  • (modified) flang/test/HLFIR/c_ptr_byvalue.f90 (+2-1)
  • (modified) flang/test/HLFIR/call_with_poly_dummy.f90 (+2-1)
  • (modified) flang/test/HLFIR/optional_dummy.f90 (+1-1)
  • (modified) flang/test/HLFIR/order_assignments/where-scheduling.f90 (+5-5)
  • (modified) flang/test/Lower/CUDA/cuda-data-attribute.cuf (+4-4)
  • (modified) flang/test/Lower/HLFIR/actual_target_for_dummy_pointer.f90 (+11-11)
  • (modified) flang/test/Lower/HLFIR/allocatable-and-pointer-status-change.f90 (+6-6)
  • (modified) flang/test/Lower/HLFIR/allocatables-and-pointers.f90 (+9-9)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-elemental-nested.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-elemental.f90 (+6-5)
  • (modified) flang/test/Lower/HLFIR/array-ctor-as-inlined-temp.f90 (+6-6)
  • (modified) flang/test/Lower/HLFIR/array-ctor-index.f90 (+4-4)
  • (modified) flang/test/Lower/HLFIR/assignment-intrinsics.f90 (+23-23)
  • (modified) flang/test/Lower/HLFIR/assumed-rank-iface-alloc-ptr.f90 (+5-5)
  • (modified) flang/test/Lower/HLFIR/assumed-rank-iface.f90 (+9-9)
  • (modified) flang/test/Lower/HLFIR/binary-ops.f90 (+42-42)
  • (modified) flang/test/Lower/HLFIR/bindc-value-derived.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/call-sequence-associated-descriptors.f90 (+8-8)
  • (modified) flang/test/Lower/HLFIR/calls-assumed-shape.f90 (+7-7)
  • (modified) flang/test/Lower/HLFIR/calls-constant-expr-arg.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/calls-f77.f90 (+7-7)
  • (modified) flang/test/Lower/HLFIR/calls-optional.f90 (+7-7)
  • (modified) flang/test/Lower/HLFIR/calls-percent-val-ref.f90 (+6-6)
  • (modified) flang/test/Lower/HLFIR/calls-poly-to-assumed-type.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/char_extremum.f03 (+20-20)
  • (modified) flang/test/Lower/HLFIR/charconvert.f90 (+4-4)
  • (modified) flang/test/Lower/HLFIR/convert-mbox-to-value.f90 (+8-8)
  • (modified) flang/test/Lower/HLFIR/convert-variable-block.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/convert-variable.f90 (+10-10)
  • (modified) flang/test/Lower/HLFIR/cray-pointers.f90 (+5-5)
  • (modified) flang/test/Lower/HLFIR/custom-intrinsic.f90 (+52-51)
  • (modified) flang/test/Lower/HLFIR/designators-component-ref.f90 (+4-4)
  • (modified) flang/test/Lower/HLFIR/designators.f90 (+21-21)
  • (modified) flang/test/Lower/HLFIR/dot_product.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/elemental-array-ops.f90 (+6-6)
  • (modified) flang/test/Lower/HLFIR/elemental-polymorphic-merge.f90 (+4-4)
  • (modified) flang/test/Lower/HLFIR/elemental-user-procedure-ref.f90 (+3-3)
  • (modified) flang/test/Lower/HLFIR/expr-addr.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/expr-box.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/expr-value.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/ignore-rank-unlimited-polymorphic.f90 (+5-5)
  • (modified) flang/test/Lower/HLFIR/implicit-type-conversion.f90 (+14-14)
  • (modified) flang/test/Lower/HLFIR/intentout-allocatable-components.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/internal-procedures.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/intrinsic-dynamically-optional.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/issue80884.f90 (+2-2)
  • (modified) flang/test/Lower/HLFIR/maxloc.f90 (+6-5)
  • (modified) flang/test/Lower/HLFIR/minloc.f90 (+6-5)
  • (modified) flang/test/Lower/HLFIR/procedure-pointer.f90 (+3-3)
  • (modified) flang/test/Lower/HLFIR/statement-functions.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/structure-constructor.f90 (+8-8)
  • (modified) flang/test/Lower/HLFIR/transformational.f90 (+1-1)
  • (modified) flang/test/Lower/HLFIR/transpose.f90 (+5-5)
  • (modified) flang/test/Lower/HLFIR/unary-ops.f90 (+6-6)
  • (modified) flang/test/Lower/HLFIR/user-defined-assignment.f90 (+19-19)
  • (modified) flang/test/Lower/HLFIR/vector-subscript-as-value.f90 (+3-3)
  • (modified) flang/test/Lower/Intrinsics/associated-proc-pointers.f90 (+6-6)
  • (modified) flang/test/Lower/Intrinsics/c_f_procpointer.f90 (+4-4)
  • (modified) flang/test/Lower/Intrinsics/c_funloc-proc-pointers.f90 (+2-2)
  • (modified) flang/test/Lower/Intrinsics/c_ptr_eq_ne.f90 (+4-4)
  • (modified) flang/test/Lower/Intrinsics/execute_command_line-optional.f90 (+6-5)
  • (modified) flang/test/Lower/Intrinsics/execute_command_line.f90 (+9-7)
  • (modified) flang/test/Lower/Intrinsics/ieee_logb.f90 (+1-1)
  • (modified) flang/test/Lower/Intrinsics/product.f90 (+1-1)
  • (modified) flang/test/Lower/Intrinsics/signal.f90 (+1-1)
  • (modified) flang/test/Lower/Intrinsics/sizeof.f90 (+2-2)
  • (modified) flang/test/Lower/Intrinsics/sum.f90 (+1-1)
  • (modified) flang/test/Lower/Intrinsics/system-optional.f90 (+3-2)
  • (modified) flang/test/Lower/Intrinsics/system.f90 (+6-4)
  • (modified) flang/test/Lower/OpenACC/acc-atomic-update-array.f90 (+9-9)
  • (modified) flang/test/Lower/OpenACC/acc-bounds.f90 (+6-6)
  • (modified) flang/test/Lower/OpenACC/acc-declare.f90 (+9-9)
  • (modified) flang/test/Lower/OpenACC/acc-loop-exit.f90 (+3-3)
  • (modified) flang/test/Lower/OpenACC/acc-private.f90 (+5-5)
  • (modified) flang/test/Lower/OpenACC/acc-reduction.f90 (+5-5)
  • (modified) flang/test/Lower/OpenMP/allocatable-array-bounds.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/array-bounds.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/flush.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/parallel-firstprivate-clause-scalar.f90 (+21-21)
  • (modified) flang/test/Lower/OpenMP/parallel-lastprivate-clause-scalar.f90 (+9-9)
  • (modified) flang/test/Lower/OpenMP/parallel-private-clause-fixes.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/parallel-private-clause-str.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/parallel-reduction3.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/parallel-wsloop-firstpriv.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/parallel-wsloop.f90 (+11-11)
  • (modified) flang/test/Lower/OpenMP/sections.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/simd.f90 (+7-7)
  • (modified) flang/test/Lower/OpenMP/single.f90 (+6-6)
  • (modified) flang/test/Lower/OpenMP/target.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-array-assumed-shape.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-iand-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-iand.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ieor-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ieor.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ior-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-ior.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-and-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-and.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-eqv-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-eqv.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-neqv-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-neqv.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-or-byref.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-logical-or.f90 (+3-3)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-byref.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-hlfir-byref.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max-hlfir.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-max.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min-byref.f90 (+2-2)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+2-2)
  • (modified) flang/test/Lower/allocatable-polymorphic.f90 (+3-3)
  • (modified) flang/test/Lower/array-expression.f90 (+1-1)
  • (modified) flang/test/Lower/character-substrings.f90 (+2-2)
  • (modified) flang/test/Lower/charconvert.f90 (+4-4)
  • (modified) flang/test/Lower/dispatch.f90 (+15-15)
  • (modified) flang/test/Lower/do_loop.f90 (+1)
  • (modified) flang/test/Lower/pointer-references.f90 (+1-1)
  • (modified) flang/test/Lower/polymorphic.f90 (+1-1)
  • (modified) flang/test/Lower/select-type.f90 (+1-1)
  • (modified) flang/test/Lower/structure-constructors-alloc-comp.f90 (+3-3)
diff --git a/flang/include/flang/Lower/AbstractConverter.h b/flang/include/flang/Lower/AbstractConverter.h
index 1cb6bcb1f5d257..0bc68de6938da4 100644
--- a/flang/include/flang/Lower/AbstractConverter.h
+++ b/flang/include/flang/Lower/AbstractConverter.h
@@ -219,6 +219,18 @@ class AbstractConverter {
   /// function.
   virtual void bindHostAssocTuple(mlir::Value val) = 0;
 
+  /// Returns fir.dummy_scope operation's result value to be used
+  /// as dummy_scope operand of hlfir.declare operations for the dummy
+  /// arguments of this function.
+  virtual mlir::Value dummyArgsScopeValue() const = 0;
+
+  /// Returns true if the given symbol is a dummy argument of this function.
+  /// Note that it returns false for all the symbols after all the variables
+  /// are instantiated for this function, i.e. it can only be used reliably
+  /// during the instatiation of the variables.
+  virtual bool
+  isRegisteredDummySymbol(Fortran::semantics::SymbolRef symRef) const = 0;
+
   //===--------------------------------------------------------------------===//
   // Types
   //===--------------------------------------------------------------------===//
diff --git a/flang/include/flang/Optimizer/Builder/HLFIRTools.h b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
index 6c36f7e84db688..738117a2a61860 100644
--- a/flang/include/flang/Optimizer/Builder/HLFIRTools.h
+++ b/flang/include/flang/Optimizer/Builder/HLFIRTools.h
@@ -238,6 +238,7 @@ fir::FortranVariableOpInterface
 genDeclare(mlir::Location loc, fir::FirOpBuilder &builder,
            const fir::ExtendedValue &exv, llvm::StringRef name,
            fir::FortranVariableFlagsAttr flags,
+           mlir::Value dummyScope = nullptr,
            fir::CUDADataAttributeAttr cudaAttr = {});
 
 /// Generate an hlfir.associate to build a variable from an expression value.
diff --git a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
index ee3c26800ae3a9..9558a6832972f2 100644
--- a/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
+++ b/flang/include/flang/Optimizer/HLFIR/HLFIROps.td
@@ -104,6 +104,7 @@ def hlfir_DeclareOp : hlfir_Op<"declare", [AttrSizedOperandSegments,
   let builders = [
     OpBuilder<(ins "mlir::Value":$memref, "llvm::StringRef":$uniq_name,
       CArg<"mlir::Value", "{}">:$shape, CArg<"mlir::ValueRange", "{}">:$typeparams,
+      CArg<"mlir::Value", "{}">:$dummy_scope,
       CArg<"fir::FortranVariableFlagsAttr", "{}">:$fortran_attrs,
       CArg<"fir::CUDADataAttributeAttr", "{}">:$cuda_attr)>];
 
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index ae8679afc603f8..1e4d824c0d1e23 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -900,6 +900,16 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     hostAssocTuple = val;
   }
 
+  mlir::Value dummyArgsScopeValue() const override final {
+    return dummyArgsScope;
+  }
+
+  bool isRegisteredDummySymbol(
+      Fortran::semantics::SymbolRef symRef) const override final {
+    auto *sym = &*symRef;
+    return registeredDummySymbols.contains(sym);
+  }
+
   void registerTypeInfo(mlir::Location loc,
                         Fortran::lower::SymbolRef typeInfoSym,
                         const Fortran::semantics::DerivedTypeSpec &typeSpec,
@@ -1145,10 +1155,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   /// yet. The final mapping will be done using this pre-mapping in
   /// Fortran::lower::mapSymbolAttributes.
   bool mapBlockArgToDummyOrResult(const Fortran::semantics::SymbolRef sym,
-                                  mlir::Value val, bool forced = false) {
-    if (!forced && lookupSymbol(sym))
-      return false;
-    localSymbols.addSymbol(sym, val, forced);
+                                  mlir::Value val, bool isResult) {
+    localSymbols.addSymbol(sym, val);
+    if (!isResult)
+      registerDummySymbol(sym);
+
     return true;
   }
 
@@ -4560,7 +4571,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
                             const Fortran::lower::CalleeInterface &callee) {
     assert(builder && "require a builder object at this point");
     using PassBy = Fortran::lower::CalleeInterface::PassEntityBy;
-    auto mapPassedEntity = [&](const auto arg) {
+    auto mapPassedEntity = [&](const auto arg, bool isResult = false) {
       if (arg.passBy == PassBy::AddressAndLength) {
         if (callee.characterize().IsBindC())
           return;
@@ -4570,10 +4581,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         fir::factory::CharacterExprHelper charHelp{*builder, loc};
         mlir::Value box =
             charHelp.createEmboxChar(arg.firArgument, arg.firLength);
-        mapBlockArgToDummyOrResult(arg.entity->get(), box);
+        mapBlockArgToDummyOrResult(arg.entity->get(), box, isResult);
       } else {
         if (arg.entity.has_value()) {
-          mapBlockArgToDummyOrResult(arg.entity->get(), arg.firArgument);
+          mapBlockArgToDummyOrResult(arg.entity->get(), arg.firArgument,
+                                     isResult);
         } else {
           assert(funit.parentHasTupleHostAssoc() && "expect tuple argument");
         }
@@ -4582,15 +4594,19 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     for (const Fortran::lower::CalleeInterface::PassedEntity &arg :
          callee.getPassedArguments())
       mapPassedEntity(arg);
+    if (lowerToHighLevelFIR() && !callee.getPassedArguments().empty()) {
+      mlir::Value scopeOp = builder->create<fir::DummyScopeOp>(toLocation());
+      setDummyArgsScope(scopeOp);
+    }
     if (std::optional<Fortran::lower::CalleeInterface::PassedEntity>
             passedResult = callee.getPassedResult()) {
-      mapPassedEntity(*passedResult);
+      mapPassedEntity(*passedResult, /*isResult=*/true);
       // FIXME: need to make sure things are OK here. addSymbol may not be OK
       if (funit.primaryResult &&
           passedResult->entity->get() != *funit.primaryResult)
         mapBlockArgToDummyOrResult(
-            *funit.primaryResult,
-            getSymbolAddress(passedResult->entity->get()));
+            *funit.primaryResult, getSymbolAddress(passedResult->entity->get()),
+            /*isResult=*/true);
     }
   }
 
@@ -4767,7 +4783,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
       Fortran::lower::StatementContext stmtCtx;
       if (std::optional<Fortran::lower::CalleeInterface::PassedEntity>
               passedResult = callee.getPassedResult()) {
-        mapBlockArgToDummyOrResult(altResult.getSymbol(), resultArg.getAddr());
+        mapBlockArgToDummyOrResult(altResult.getSymbol(), resultArg.getAddr(),
+                                   /*isResult=*/true);
         Fortran::lower::mapSymbolAttributes(*this, altResult, localSymbols,
                                             stmtCtx);
       } else {
@@ -4811,6 +4828,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     if (!funit.getHostAssoc().empty())
       funit.getHostAssoc().hostProcedureBindings(*this, localSymbols);
 
+    // Unregister all dummy symbols, so that their cloning (e.g. for OpenMP
+    // privatization) does not create the cloned hlfir.declare operations
+    // with dummy_scope operands.
+    resetRegisteredDummySymbols();
+
     // Create most function blocks in advance.
     createEmptyBlocks(funit.evaluationList);
 
@@ -4930,6 +4952,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     hostAssocTuple = mlir::Value{};
     localSymbols.clear();
     blockId = 0;
+    dummyArgsScope = mlir::Value{};
+    resetRegisteredDummySymbols();
   }
 
   /// Helper to generate GlobalOps when the builder is not positioned in any
@@ -4958,6 +4982,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     delete builder;
     builder = nullptr;
     localSymbols.clear();
+    resetRegisteredDummySymbols();
   }
 
   /// Instantiate the data from a BLOCK DATA unit.
@@ -5375,6 +5400,23 @@ class FirConverter : public Fortran::lower::AbstractConverter {
                                         globalOmpRequiresSymbol);
   }
 
+  /// Record fir.dummy_scope operation for this function.
+  /// It will be used to set dummy_scope operand of the hlfir.declare
+  /// operations.
+  void setDummyArgsScope(mlir::Value val) {
+    assert(!dummyArgsScope && val);
+    dummyArgsScope = val;
+  }
+
+  /// Record the given symbol as a dummy argument of this function.
+  void registerDummySymbol(Fortran::semantics::SymbolRef symRef) {
+    auto *sym = &*symRef;
+    registeredDummySymbols.insert(sym);
+  }
+
+  /// Reset all registered dummy symbols.
+  void resetRegisteredDummySymbols() { registeredDummySymbols.clear(); }
+
   //===--------------------------------------------------------------------===//
 
   Fortran::lower::LoweringBridge &bridge;
@@ -5401,6 +5443,15 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   /// Tuple of host associated variables
   mlir::Value hostAssocTuple;
 
+  /// Value of fir.dummy_scope operation for this function.
+  mlir::Value dummyArgsScope;
+
+  /// A set of dummy argument symbols for this function.
+  /// The set is only preserved during the instatiation
+  /// of variables for this function.
+  llvm::SmallPtrSet<const Fortran::semantics::Symbol *, 16>
+      registeredDummySymbols;
+
   /// A map of unique names for constant expressions.
   /// The names are used for representing the constant expressions
   /// with global constant initialized objects.
diff --git a/flang/lib/Lower/ConvertArrayConstructor.cpp b/flang/lib/Lower/ConvertArrayConstructor.cpp
index a5b5838fe6b621..341fad9a5e43c7 100644
--- a/flang/lib/Lower/ConvertArrayConstructor.cpp
+++ b/flang/lib/Lower/ConvertArrayConstructor.cpp
@@ -318,7 +318,7 @@ class RuntimeTempStrategy : public StrategyBase {
       mlir::Value shape = builder.genShape(loc, extents);
       declare = builder.create<hlfir::DeclareOp>(
           loc, tempStorage, tempName, shape, lengths,
-          fir::FortranVariableFlagsAttr{});
+          /*dummy_scope=*/nullptr, fir::FortranVariableFlagsAttr{});
       initialBoxValue =
           builder.createBox(loc, boxType, declare->getOriginalBase(), shape,
                             /*slice=*/mlir::Value{}, lengths, /*tdesc=*/{});
diff --git a/flang/lib/Lower/ConvertExprToHLFIR.cpp b/flang/lib/Lower/ConvertExprToHLFIR.cpp
index 93bdf650f9ffbe..3c305955520e22 100644
--- a/flang/lib/Lower/ConvertExprToHLFIR.cpp
+++ b/flang/lib/Lower/ConvertExprToHLFIR.cpp
@@ -1676,7 +1676,8 @@ class HlfirBuilder {
     mlir::Value storagePtr = builder.createTemporary(loc, recTy);
     auto varOp = hlfir::EntityWithAttributes{builder.create<hlfir::DeclareOp>(
         loc, storagePtr, "ctor.temp", /*shape=*/nullptr,
-        /*typeparams=*/mlir::ValueRange{}, fir::FortranVariableFlagsAttr{})};
+        /*typeparams=*/mlir::ValueRange{}, /*dummy_scope=*/nullptr,
+        fir::FortranVariableFlagsAttr{})};
 
     // Initialize any components that need initialization.
     mlir::Value box = builder.createBox(loc, fir::ExtendedValue{varOp});
diff --git a/flang/lib/Lower/ConvertVariable.cpp b/flang/lib/Lower/ConvertVariable.cpp
index f31fbab41028c1..5ddd8a6a9d41f2 100644
--- a/flang/lib/Lower/ConvertVariable.cpp
+++ b/flang/lib/Lower/ConvertVariable.cpp
@@ -1683,7 +1683,8 @@ static void genDeclareSymbol(Fortran::lower::AbstractConverter &converter,
 
       // Declare a local pointer variable.
       auto newBase = builder.create<hlfir::DeclareOp>(
-          loc, boxAlloc, name, /*shape=*/nullptr, lenParams, attributes);
+          loc, boxAlloc, name, /*shape=*/nullptr, lenParams,
+          /*dummy_scope=*/nullptr, attributes);
       mlir::Value nullAddr = builder.createNullConstant(
           loc, llvm::cast<fir::BaseBoxType>(ptrBoxType).getEleTy());
 
@@ -1710,8 +1711,12 @@ static void genDeclareSymbol(Fortran::lower::AbstractConverter &converter,
       symMap.addVariableDefinition(sym, newBase, force);
       return;
     }
+    mlir::Value dummyScope;
+    if (converter.isRegisteredDummySymbol(sym))
+      dummyScope = converter.dummyArgsScopeValue();
     auto newBase = builder.create<hlfir::DeclareOp>(
-        loc, base, name, shapeOrShift, lenParams, attributes, cudaAttr);
+        loc, base, name, shapeOrShift, lenParams, dummyScope, attributes,
+        cudaAttr);
     symMap.addVariableDefinition(sym, newBase, force);
     return;
   }
@@ -1761,8 +1766,11 @@ void Fortran::lower::genDeclareSymbol(
         Fortran::lower::translateSymbolCUDADataAttribute(builder.getContext(),
                                                          sym.GetUltimate());
     auto name = converter.mangleName(sym);
-    hlfir::EntityWithAttributes declare =
-        hlfir::genDeclare(loc, builder, exv, name, attributes, cudaAttr);
+    mlir::Value dummyScope;
+    if (converter.isRegisteredDummySymbol(sym))
+      dummyScope = converter.dummyArgsScopeValue();
+    hlfir::EntityWithAttributes declare = hlfir::genDeclare(
+        loc, builder, exv, name, attributes, dummyScope, cudaAttr);
     symMap.addVariableDefinition(sym, declare.getIfVariableInterface(), force);
     return;
   }
@@ -2022,7 +2030,9 @@ void Fortran::lower::mapSymbolAttributes(
           fir::factory::genMutableBoxRead(
               builder, loc,
               fir::factory::createTempMutableBox(builder, loc, ty, {}, {},
-                                                 isPolymorphic)));
+                                                 isPolymorphic)),
+          fir::FortranVariableFlagsEnum::None,
+          converter.isRegisteredDummySymbol(sym));
       return true;
     }
     return false;
diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index eae2afc760e648..b02e7be75d20fe 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -425,7 +425,8 @@ static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
       auto alloca = builder.create<fir::AllocaOp>(loc, refTy.getEleTy());
       auto declareOp = builder.create<hlfir::DeclareOp>(
           loc, alloca, accPrivateInitName, /*shape=*/nullptr,
-          llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+          llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+          fir::FortranVariableFlagsAttr{});
       retVal = declareOp.getBase();
     } else if (auto seqTy = mlir::dyn_cast_or_null<fir::SequenceType>(
                    refTy.getEleTy())) {
@@ -446,7 +447,8 @@ static void genPrivateLikeInitRegion(mlir::OpBuilder &builder, RecipeOp recipe,
             loc, seqTy, /*typeparams=*/mlir::ValueRange{}, extents);
         auto declareOp = builder.create<hlfir::DeclareOp>(
             loc, alloca, accPrivateInitName, shape,
-            llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+            llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+            fir::FortranVariableFlagsAttr{});
         retVal = declareOp.getBase();
       }
     }
@@ -666,10 +668,12 @@ mlir::acc::FirstprivateRecipeOp Fortran::lower::createOrGetFirstprivateRecipe(
 
     auto leftDeclOp = builder.create<hlfir::DeclareOp>(
         loc, recipe.getCopyRegion().getArgument(0), llvm::StringRef{}, shape,
-        llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+        llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+        fir::FortranVariableFlagsAttr{});
     auto rightDeclOp = builder.create<hlfir::DeclareOp>(
         loc, recipe.getCopyRegion().getArgument(1), llvm::StringRef{}, shape,
-        llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+        llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+        fir::FortranVariableFlagsAttr{});
 
     hlfir::DesignateOp::Subscripts triplets =
         getSubscriptsFromArgs(recipe.getCopyRegion().getArguments());
@@ -975,7 +979,8 @@ static mlir::Value genReductionInitRegion(fir::FirOpBuilder &builder,
     mlir::Value alloca = builder.create<fir::AllocaOp>(loc, ty);
     auto declareOp = builder.create<hlfir::DeclareOp>(
         loc, alloca, accReductionInitName, /*shape=*/nullptr,
-        llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+        llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+        fir::FortranVariableFlagsAttr{});
     builder.create<fir::StoreOp>(loc, builder.createConvert(loc, ty, initValue),
                                  declareOp.getBase());
     return declareOp.getBase();
@@ -991,7 +996,8 @@ static mlir::Value genReductionInitRegion(fir::FirOpBuilder &builder,
           loc, seqTy, /*typeparams=*/mlir::ValueRange{}, extents);
       auto declareOp = builder.create<hlfir::DeclareOp>(
           loc, alloca, accReductionInitName, shape,
-          llvm::ArrayRef<mlir::Value>{}, fir::FortranVariableFlagsAttr{});
+          llvm::ArrayRef<mlir::Value>{}, /*dummy_scope=*/nullptr,
+          fir::FortranVariableFlagsAttr{});
       mlir::Type idxTy = builder.getIndexType();
       mlir::Type refTy = fir::ReferenceType::get(seqTy.getEleTy());
       llvm::SmallVector<fir::DoLoopOp> loops;
@@ -1143,10 +1149,10 @@ static void genCombiner(fir::FirOpBuilder &builder, mlir::Location loc,
                                    recipe.getCombinerRegion().getArguments());
       auto v1DeclareOp = builder.create<hlfir::DeclareOp>(
           loc, value1, llvm::StringRef{}, shape, llvm::ArrayRef<mlir::Value>{},
-          fir::FortranVariableFlagsAttr{});
+          /*dummy_scope=*/nullptr, fir::FortranVariableFlagsAttr{});
       auto v2DeclareOp = builder.create<hlfir::DeclareOp>(
           loc, value2, llvm::StringRef{}, shape, llvm::ArrayRef<mlir::Value>{},
-          fir::FortranVariableFlagsAttr{});
+          /*dummy_scope=*/nullptr, fir::FortranVariableFlagsAttr{});
       hlfir::DesignateOp::Subscripts triplets = getTripletsFromArgs(recipe);
 
       llvm::SmallVector<mlir::Value> lenParamsLeft;
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 79525d6dfe7a21..d22c23de135003 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -650,12 +650,12 @@ createCopyFunc(mlir::Location loc, Fortran::lower::AbstractConverter &converter,
           builder.createIntegerConstant(loc, builder.getIndexType(), extent));
     shape = builder.create<fir::ShapeOp>(loc, extents);
   }
-  auto declDst = builder.create<hlfir::DeclareOp>(loc, funcOp.getArgument(0),
-                                                  copyFuncName + "_dst", shape,
-                                                  typeparams, attrs);
-  auto declSrc = builder.create<hlfir::DeclareOp>(loc, funcOp.getArgument(1),
-                                                  copyFuncName + "_src", shape,
-                                                  typeparams, attrs);
+  auto declDst = builder.create<hlfir::DeclareOp>(
+      loc, funcOp.getArgument(0), copyFuncName + "_dst", shape, typeparams,
+      /*dummy_scope=*/nullptr, attrs);
+  auto declSrc = builder.create<hlfir::DeclareOp>(
+      loc, funcOp.getArgument(1), copyFuncName + "_src", shape, typeparams,
+      /*dummy_scope=*/nullptr, attrs);
   converter.copyVar(loc, declDst.getBase(), declSrc.getBase());
   builder.create<mlir::func::ReturnOp>(loc);
   return funcOp;
diff --git a/flang/lib/Optimizer/Builder/HLFIRTools.cpp b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
index 44779427ab557a..86d42f19189412 100644
--- a/flang/lib/Optimizer/Builder/HLFIRTools.cpp
+++ b/flang/lib/Optimizer/Builder/HLFIRTools.cpp
@@ -198,7 +198,7 @@ mlir::Value hlfir::Entity::getFirBase() const {
 fir::FortranVariableOpInterface
 hlfir::genDeclare(mlir::Location loc, fir::FirOpBuilder &builder,
                   const fir::ExtendedValue &exv, llvm::StringRef name,
-                  fir::FortranVariableFlagsAttr flags,
+                  fir::FortranVariableFlagsAttr flags, mlir::Value dummyScope,
                   fir::CUDADataAttributeAttr cudaAttr) {
 
   mlir::Value base = fir::getBase(exv);
@@ -229,7 +229,7 @@ hlfir::genDeclare(mlir::Location loc, fir::FirOpBuilder &builder,
       },
       [](const auto &) {});
   auto declareOp = builder.create<hlfir::DeclareOp>(
-      loc, base, name, shapeOrShift, lenParams, flags, cudaAttr);
+      loc, base, name, shapeOrShift, ...
[truncated]

@vzakhari vzakhari force-pushed the dummy_scope_lowering branch from 58a8230 to fe8c842 Compare May 6, 2024 19:57
@vzakhari vzakhari requested a review from jeanPerier May 8, 2024 15:24
vzakhari added 2 commits May 8, 2024 08:24
The lowering produces fir.dummy_scope operation if the current
function has dummy arguments. Each hlfir.declare generated
for a dummy argument is then using the result of fir.dummy_scope
as its dummy_scope operand. This is only done for HLFIR.

I was not able to find a reliable way to identify dummy symbols
in `genDeclareSymbol`, so I added a set of registered dummy symbols
that is alive during the variables instantiation for the current
function. The set is initialized during the mapping of the dummy
argument symbols to their MLIR values. It is reset right after
all variables are instantiated - this is done to avoid generating
hlfir.declare operations with dummy_scope for the clones of
the dummy symbols (e.g. this happens with OpenMP privatization).

If this can be done in a cleaner way, please advise.
@vzakhari vzakhari force-pushed the dummy_scope_lowering branch from fe8c842 to 6c0856f Compare May 8, 2024 16:40
@vzakhari vzakhari merged commit 1710c8c into llvm:main May 8, 2024
3 of 4 checks passed
JumpMasterJJ added a commit that referenced this pull request May 17, 2024
…ion (#92571)

This is same as #90578 with an
added fix. This PR updated tests of etime intrinsic due to Lowering
changes for assigning dummy_scope to hlfir.declare. Referring to
#92472 and
#90989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants