debugging failure

Author: akuhlens

flang/include/flang/Lower/OpenACC.h

@@ -37,11 +37,16 @@ class FirOpBuilder;
 }
 
 namespace Fortran {
+namespace evaluate {
+class ProcedureDesignator;
+} // namespace evaluate
+
 namespace parser {
 struct AccClauseList;
 struct OpenACCConstruct;
 struct OpenACCDeclarativeConstruct;
 struct OpenACCRoutineConstruct;
+struct ProcedureDesignator;
 } // namespace parser
 
 namespace semantics {
@@ -71,6 +76,8 @@ static constexpr llvm::StringRef declarePostDeallocSuffix =
 
 static constexpr llvm::StringRef privatizationRecipePrefix = "privatization";
 
+bool needsOpenACCRoutineConstruct(const Fortran::evaluate::ProcedureDesignator *);
+
 mlir::Value genOpenACCConstruct(AbstractConverter &,
                                 Fortran::semantics::SemanticsContext &,
                                 pft::Evaluation &,

flang/include/flang/Semantics/symbol.h

@@ -22,6 +22,7 @@
 #include <list>
 #include <optional>
 #include <set>
+#include <variant>
 #include <vector>
 
 namespace llvm {
@@ -139,25 +140,26 @@ class OpenACCRoutineDeviceTypeInfo {
   void set_isGang(bool value = true) { isGang_ = value; }
   unsigned gangDim() const { return gangDim_; }
   void set_gangDim(unsigned value) { gangDim_ = value; }
-  const std::string *bindName() const {
-    return bindName_ ? &*bindName_ : nullptr;
+  const std::variant<std::string, SymbolRef> *bindName() const {
+    return bindName_.has_value() ? &*bindName_ : nullptr;
   }
-  bool bindNameIsInternal() const {return bindNameIsInternal_;}
-  void set_bindName(std::string &&name, bool isInternal=false) { 
-    bindName_ = std::move(name); 
-    bindNameIsInternal_ = isInternal;
+  const std::optional<std::variant<std::string, SymbolRef>> &bindNameOpt() const {
+    return bindName_;
   }
+  void set_bindName(std::string &&name) { bindName_.emplace(std::move(name)); }
+  void set_bindName(SymbolRef symbol) { bindName_.emplace(symbol); }
 
   Fortran::common::OpenACCDeviceType dType() const { return deviceType_; }
 
+  friend llvm::raw_ostream &operator<<(
+      llvm::raw_ostream &, const OpenACCRoutineDeviceTypeInfo &);
 private:
   bool isSeq_{false};
   bool isVector_{false};
   bool isWorker_{false};
   bool isGang_{false};
   unsigned gangDim_{0};
-  std::optional<std::string> bindName_;
-  bool bindNameIsInternal_{false};
+  std::optional<std::variant<std::string, SymbolRef>> bindName_;
   Fortran::common::OpenACCDeviceType deviceType_{
       Fortran::common::OpenACCDeviceType::None};
 };
@@ -187,6 +189,9 @@ class OpenACCRoutineInfo : public OpenACCRoutineDeviceTypeInfo {
     return deviceTypeInfos_.back();
   }
 
+  friend llvm::raw_ostream &operator<<(
+      llvm::raw_ostream &, const OpenACCRoutineInfo &);
+
 private:
   std::list<OpenACCRoutineDeviceTypeInfo> deviceTypeInfos_;
   bool isNohost_{false};

flang/lib/Lower/Bridge.cpp

@@ -438,14 +438,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
               [&](Fortran::lower::pft::ModuleLikeUnit &m) { lowerMod(m); },
               [&](Fortran::lower::pft::BlockDataUnit &b) {},
               [&](Fortran::lower::pft::CompilerDirectiveUnit &d) {},
-              [&](Fortran::lower::pft::OpenACCDirectiveUnit &d) {
-                builder = new fir::FirOpBuilder(
-                    bridge.getModule(), bridge.getKindMap(), &mlirSymbolTable);
-                Fortran::lower::genOpenACCRoutineConstruct(
-                    *this, bridge.getSemanticsContext(), bridge.getModule(),
-                    d.routine);
-                builder = nullptr;
-              },
+              [&](Fortran::lower::pft::OpenACCDirectiveUnit &d) {},
           },
           u);
     }
@@ -472,6 +465,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
   /// Declare a function.
   void declareFunction(Fortran::lower::pft::FunctionLikeUnit &funit) {
     setCurrentPosition(funit.getStartingSourceLoc());
+    builder = new fir::FirOpBuilder(
+        bridge.getModule(), bridge.getKindMap(), &mlirSymbolTable);
     for (int entryIndex = 0, last = funit.entryPointList.size();
          entryIndex < last; ++entryIndex) {
       funit.setActiveEntry(entryIndex);
@@ -498,6 +493,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     for (Fortran::lower::pft::ContainedUnit &unit : funit.containedUnitList)
       if (auto *f = std::get_if<Fortran::lower::pft::FunctionLikeUnit>(&unit))
         declareFunction(*f);
+    builder = nullptr;
   }
 
   /// Get the scope that is defining or using \p sym. The returned scope is not
@@ -1035,7 +1031,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     return bridge.getSemanticsContext().FindScope(currentPosition);
   }
 
-  fir::FirOpBuilder &getFirOpBuilder() override final { return *builder; }
+  fir::FirOpBuilder &getFirOpBuilder() override final { 
+    CHECK(builder && "builder is not set before calling getFirOpBuilder");
+    return *builder; 
+  }
 
   mlir::ModuleOp getModuleOp() override final { return bridge.getModule(); }
 
@@ -5617,6 +5616,10 @@ class FirConverter : public Fortran::lower::AbstractConverter {
     LLVM_DEBUG(llvm::dbgs() << "\n[bridge - startNewFunction]";
                if (auto *sym = scope.symbol()) llvm::dbgs() << " " << *sym;
                llvm::dbgs() << "\n");
+    // I don't think setting the builder is necessary here, because callee 
+    // always looks up the FuncOp from the module. If there was a function that
+    // was not declared yet. This call to callee will cause an assertion
+    //failure.
     Fortran::lower::CalleeInterface callee(funit, *this);
     mlir::func::FuncOp func = callee.addEntryBlockAndMapArguments();
     builder =

flang/lib/Lower/CallInterface.cpp

@@ -10,6 +10,7 @@
 #include "flang/Evaluate/fold.h"
 #include "flang/Lower/Bridge.h"
 #include "flang/Lower/Mangler.h"
+#include "flang/Lower/OpenACC.h"
 #include "flang/Lower/PFTBuilder.h"
 #include "flang/Lower/StatementContext.h"
 #include "flang/Lower/Support/Utils.h"
@@ -20,6 +21,7 @@
 #include "flang/Optimizer/Dialect/FIROpsSupport.h"
 #include "flang/Optimizer/Support/InternalNames.h"
 #include "flang/Optimizer/Support/Utils.h"
+#include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/symbol.h"
 #include "flang/Semantics/tools.h"
 #include "flang/Support/Fortran.h"
@@ -715,6 +717,14 @@ void Fortran::lower::CallInterface<T>::declare() {
           func.setArgAttrs(placeHolder.index(), placeHolder.value().attributes);
 
       setCUDAAttributes(func, side().getProcedureSymbol(), characteristic);
+      
+      if (const Fortran::semantics::Symbol *sym = side().getProcedureSymbol()) {
+        if (const auto &info{sym->GetUltimate().detailsIf<Fortran::semantics::SubprogramDetails>()}) {
+          if (!info->openACCRoutineInfos().empty()) {
+            genOpenACCRoutineConstruct(converter, module, func, info->openACCRoutineInfos());
+          }
+        }
+      }
     }
   }
 }
@@ -1688,17 +1698,8 @@ class SignatureBuilder
       fir::emitFatalError(converter.getCurrentLocation(),
                           "SignatureBuilder should only be used once");
     declare();
+
     interfaceDetermined = true;
-    if (procDesignator && procDesignator->GetInterfaceSymbol() &&
-        procDesignator->GetInterfaceSymbol()
-            ->has<Fortran::semantics::SubprogramDetails>()) {
-      auto info = procDesignator->GetInterfaceSymbol()
-                      ->get<Fortran::semantics::SubprogramDetails>();
-      if (!info.openACCRoutineInfos().empty()) {
-        genOpenACCRoutineConstruct(converter, converter.getModuleOp(),
-                                   getFuncOp(), info.openACCRoutineInfos());
-      }
-    }
     return getFuncOp();
   }
 

flang/lib/Lower/OpenACC.cpp

@@ -36,6 +36,7 @@
 #include "llvm/Frontend/OpenACC/ACC.h.inc"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "mlir/Dialect/ControlFlow/IR/ControlFlowOps.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Support/LLVM.h"
@@ -4140,6 +4141,14 @@ static void attachRoutineInfo(mlir::func::FuncOp func,
       mlir::acc::RoutineInfoAttr::get(func.getContext(), routines));
 }
 
+static mlir::ArrayAttr getArrayAttrOrNull(fir::FirOpBuilder &builder, llvm::SmallVector<mlir::Attribute> &attributes) {
+  if (attributes.empty()) {
+    return nullptr;
+  } else {
+    return builder.getArrayAttr(attributes);
+  }
+}
+
 void createOpenACCRoutineConstruct(
     Fortran::lower::AbstractConverter &converter, mlir::Location loc,
     mlir::ModuleOp mod, mlir::func::FuncOp funcOp, std::string funcName,
@@ -4173,31 +4182,29 @@ void createOpenACCRoutineConstruct(
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
   modBuilder.create<mlir::acc::RoutineOp>(
       loc, routineOpStr, funcName,
-      bindNames.empty() ? nullptr : builder.getArrayAttr(bindNames),
-      bindNameDeviceTypes.empty() ? nullptr
-                                  : builder.getArrayAttr(bindNameDeviceTypes),
-      workerDeviceTypes.empty() ? nullptr
-                                : builder.getArrayAttr(workerDeviceTypes),
-      vectorDeviceTypes.empty() ? nullptr
-                                : builder.getArrayAttr(vectorDeviceTypes),
-      seqDeviceTypes.empty() ? nullptr : builder.getArrayAttr(seqDeviceTypes),
+      getArrayAttrOrNull(builder, bindNames),
+      getArrayAttrOrNull(builder, bindNameDeviceTypes),
+      getArrayAttrOrNull(builder, workerDeviceTypes),
+      getArrayAttrOrNull(builder, vectorDeviceTypes),
+      getArrayAttrOrNull(builder, seqDeviceTypes),
       hasNohost, /*implicit=*/false,
-      gangDeviceTypes.empty() ? nullptr : builder.getArrayAttr(gangDeviceTypes),
-      gangDimValues.empty() ? nullptr : builder.getArrayAttr(gangDimValues),
-      gangDimDeviceTypes.empty() ? nullptr
-                                 : builder.getArrayAttr(gangDimDeviceTypes));
-
-  if (funcOp)
-    attachRoutineInfo(funcOp, builder.getSymbolRefAttr(routineOpStr));
-  else
+      getArrayAttrOrNull(builder, gangDeviceTypes),
+      getArrayAttrOrNull(builder, gangDimValues),
+      getArrayAttrOrNull(builder, gangDimDeviceTypes));
+
+  auto symbolRefAttr = builder.getSymbolRefAttr(routineOpStr);
+  if (funcOp) {
+    
+    attachRoutineInfo(funcOp, symbolRefAttr);
+  } else {
     // FuncOp is not lowered yet. Keep the information so the routine info
     // can be attached later to the funcOp.
-    converter.getAccDelayedRoutines().push_back(
-        std::make_pair(funcName, builder.getSymbolRefAttr(routineOpStr)));
+    converter.getAccDelayedRoutines().push_back(std::make_pair(funcName, symbolRefAttr));
+  }
 }
 
 static void interpretRoutineDeviceInfo(
-    fir::FirOpBuilder &builder,
+    Fortran::lower::AbstractConverter &converter,
     const Fortran::semantics::OpenACCRoutineDeviceTypeInfo &dinfo,
     llvm::SmallVector<mlir::Attribute> &seqDeviceTypes,
     llvm::SmallVector<mlir::Attribute> &vectorDeviceTypes,
@@ -4207,23 +4214,24 @@ static void interpretRoutineDeviceInfo(
     llvm::SmallVector<mlir::Attribute> &gangDeviceTypes,
     llvm::SmallVector<mlir::Attribute> &gangDimValues,
     llvm::SmallVector<mlir::Attribute> &gangDimDeviceTypes) {
-  mlir::MLIRContext *context{builder.getContext()};
+  fir::FirOpBuilder &builder = converter.getFirOpBuilder();
+  auto getDeviceTypeAttr = [&]() -> mlir::Attribute {
+    auto context = builder.getContext();
+    auto value = getDeviceType(dinfo.dType());
+    return mlir::acc::DeviceTypeAttr::get(context, value );
+  };
   if (dinfo.isSeq()) {
-    seqDeviceTypes.push_back(
-        mlir::acc::DeviceTypeAttr::get(context, getDeviceType(dinfo.dType())));
+    seqDeviceTypes.push_back(getDeviceTypeAttr());
   }
   if (dinfo.isVector()) {
-    vectorDeviceTypes.push_back(
-        mlir::acc::DeviceTypeAttr::get(context, getDeviceType(dinfo.dType())));
+    vectorDeviceTypes.push_back(getDeviceTypeAttr());
   }
   if (dinfo.isWorker()) {
-    workerDeviceTypes.push_back(
-        mlir::acc::DeviceTypeAttr::get(context, getDeviceType(dinfo.dType())));
+    workerDeviceTypes.push_back(getDeviceTypeAttr());
   }
   if (dinfo.isGang()) {
     unsigned gangDim = dinfo.gangDim();
-    auto deviceType =
-        mlir::acc::DeviceTypeAttr::get(context, getDeviceType(dinfo.dType()));
+    auto deviceType = getDeviceTypeAttr();
     if (!gangDim) {
       gangDeviceTypes.push_back(deviceType);
     } else {
@@ -4232,10 +4240,18 @@ static void interpretRoutineDeviceInfo(
       gangDimDeviceTypes.push_back(deviceType);
     }
   }
-  if (const std::string *bindName{dinfo.bindName()}) {
-    bindNames.push_back(builder.getStringAttr(*bindName));
-    bindNameDeviceTypes.push_back(
-        mlir::acc::DeviceTypeAttr::get(context, getDeviceType(dinfo.dType())));
+  if (dinfo.bindNameOpt().has_value()) {
+    const auto &bindName = dinfo.bindNameOpt().value();
+    mlir::Attribute bindNameAttr;
+    if (const auto &bindStr{std::get_if<std::string>(&bindName)}) {
+        bindNameAttr = builder.getStringAttr(*bindStr);
+      } else if (const auto &bindSym{std::get_if<Fortran::semantics::SymbolRef>(&bindName)}) {
+        bindNameAttr = builder.getStringAttr(converter.mangleName(*bindSym));
+      } else {
+        llvm_unreachable("Unsupported bind name type");     
+      }
+      bindNames.push_back(bindNameAttr);
+      bindNameDeviceTypes.push_back(getDeviceTypeAttr());
   }
 }
 
@@ -4244,7 +4260,6 @@ void Fortran::lower::genOpenACCRoutineConstruct(
     mlir::func::FuncOp funcOp,
     const std::vector<Fortran::semantics::OpenACCRoutineInfo> &routineInfos) {
   CHECK(funcOp && "Expected a valid function operation");
-  fir::FirOpBuilder &builder{converter.getFirOpBuilder()};
   mlir::Location loc{funcOp.getLoc()};
   std::string funcName{funcOp.getName()};
 
@@ -4262,7 +4277,7 @@ void Fortran::lower::genOpenACCRoutineConstruct(
     }
     // Note: Device Independent Attributes are set to the
     // none device type in `info`.
-    interpretRoutineDeviceInfo(builder, info, seqDeviceTypes, vectorDeviceTypes,
+    interpretRoutineDeviceInfo(converter, info, seqDeviceTypes, vectorDeviceTypes,
                                workerDeviceTypes, bindNameDeviceTypes,
                                bindNames, gangDeviceTypes, gangDimValues,
                                gangDimDeviceTypes);
@@ -4271,7 +4286,7 @@ void Fortran::lower::genOpenACCRoutineConstruct(
     for (const Fortran::semantics::OpenACCRoutineDeviceTypeInfo &dinfo :
          info.deviceTypeInfos()) {
       interpretRoutineDeviceInfo(
-          builder, dinfo, seqDeviceTypes, vectorDeviceTypes, workerDeviceTypes,
+          converter, dinfo, seqDeviceTypes, vectorDeviceTypes, workerDeviceTypes,
           bindNameDeviceTypes, bindNames, gangDeviceTypes, gangDimValues,
           gangDimDeviceTypes);
     }
@@ -4369,8 +4384,6 @@ void Fortran::lower::genOpenACCRoutineConstruct(
                    std::get_if<Fortran::parser::AccClause::Bind>(&clause.u)) {
       if (const auto *name =
               std::get_if<Fortran::parser::Name>(&bindClause->v.u)) {
-        // FIXME: This case mangles the name, the one below does not.
-        // which is correct?
         mlir::Attribute bindNameAttr =
             builder.getStringAttr(converter.mangleName(*name->symbol));
         for (auto crtDeviceTypeAttr : crtDeviceTypes) {

flang/lib/Semantics/mod-file.cpp

@@ -24,6 +24,7 @@
 #include <fstream>
 #include <set>
 #include <string_view>
+#include <variant>
 #include <vector>
 
 namespace Fortran::semantics {
@@ -638,8 +639,14 @@ static void PutOpenACCDeviceTypeRoutineInfo(
   if (info.isWorker()) {
     os << " worker";
   }
-  if (info.bindName()) {
-    os << " bind(" << *info.bindName() << ")";
+  if (const std::variant<std::string, SymbolRef> *bindName{info.bindName()}) {
+    os << " bind(";
+    if (std::holds_alternative<std::string>(*bindName)) {
+      os << "\"" << std::get<std::string>(*bindName) << "\"";
+    } else {
+      os << std::get<SymbolRef>(*bindName)->name();
+    }
+    os << ")";
   }
 }