Revert "[Flang[[OpenMP] Initial Allocatables/Pointers (Descriptor Types) mapping PR"

This reverts commit 5a09b1e.

Author: agozillon

flang/include/flang/Optimizer/CodeGen/CodeGenOpenMP.h

@@ -321,9 +321,6 @@ bool isBoxNone(mlir::Type ty);
 /// e.g. !fir.box<!fir.type<derived>>
 bool isBoxedRecordType(mlir::Type ty);
 
-/// Return true iff `ty` is a type that contains descriptor information.
-bool isTypeWithDescriptor(mlir::Type ty);
-
 /// Return true iff `ty` is a scalar boxed record type.
 /// e.g. !fir.box<!fir.type<derived>>
 ///      !fir.box<!fir.heap<!fir.type<derived>>>

flang/include/flang/Optimizer/Dialect/FIRType.h

@@ -1864,76 +1864,30 @@ bool ClauseProcessor::processLink(
 
 static mlir::omp::MapInfoOp
 createMapInfoOp(fir::FirOpBuilder &builder, mlir::Location loc,
-                mlir::Value baseAddr, mlir::Value varPtrPtr, std::string name,
-                mlir::SmallVector<mlir::Value> bounds,
-                mlir::SmallVector<mlir::Value> members, uint64_t mapType,
-                mlir::omp::VariableCaptureKind mapCaptureType, mlir::Type retTy,
-                bool isVal = false) {
+                mlir::Value baseAddr, std::stringstream &name,
+                mlir::SmallVector<mlir::Value> bounds, uint64_t mapType,
+                mlir::omp::VariableCaptureKind mapCaptureType,
+                mlir::Type retTy) {
+  mlir::Value varPtr, varPtrPtr;
+  mlir::TypeAttr varType;
+
   if (auto boxTy = baseAddr.getType().dyn_cast<fir::BaseBoxType>()) {
     baseAddr = builder.create<fir::BoxAddrOp>(loc, baseAddr);
     retTy = baseAddr.getType();
   }
 
-  mlir::TypeAttr varType = mlir::TypeAttr::get(
+  varPtr = baseAddr;
+  varType = mlir::TypeAttr::get(
       llvm::cast<mlir::omp::PointerLikeType>(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create<mlir::omp::MapInfoOp>(
-      loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+      loc, retTy, varPtr, varType, varPtrPtr, bounds,
       builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
       builder.getAttr<mlir::omp::VariableCaptureKindAttr>(mapCaptureType),
-      builder.getStringAttr(name));
-
+      builder.getStringAttr(name.str()));
   return op;
 }
 
-static mlir::omp::MapInfoOp processDescriptorTypeMappings(
-    Fortran::semantics::SemanticsContext &semanticsContext,
-    Fortran::lower::StatementContext &stmtCtx,
-    Fortran::lower::AbstractConverter &converter, mlir::Location loc,
-    mlir::Value descriptorAddr, mlir::Value descDataBaseAddr,
-    mlir::ValueRange bounds, std::string asFortran,
-    llvm::omp::OpenMPOffloadMappingFlags mapCaptureType) {
-  llvm::SmallVector<mlir::Value> descriptorBaseAddrMembers;
-  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-
-  mlir::Value descriptor = descriptorAddr;
-
-  // The fir::BoxOffsetOp only works with !fir.ref<!fir.box<...>> types, as
-  // allowing it to access non-reference box operations can cause some
-  // problematic SSA IR. However, in the case of assumed shape's the type
-  // is not a !fir.ref, in these cases to retrieve the appropriate
-  // !fir.ref<!fir.box<...>> to access the data we need to map we must
-  // perform an alloca and then store to it and retrieve the data from the new
-  // alloca.
-  if (mlir::isa<fir::BaseBoxType>(descriptorAddr.getType())) {
-    mlir::OpBuilder::InsertPoint insPt = firOpBuilder.saveInsertionPoint();
-    firOpBuilder.setInsertionPointToStart(firOpBuilder.getAllocaBlock());
-    descriptor =
-        firOpBuilder.create<fir::AllocaOp>(loc, descriptorAddr.getType());
-    firOpBuilder.restoreInsertionPoint(insPt);
-    firOpBuilder.create<fir::StoreOp>(loc, descriptorAddr, descriptor);
-  }
-
-  mlir::Value baseAddrAddr = firOpBuilder.create<fir::BoxOffsetOp>(
-      loc, descriptor, fir::BoxFieldAttr::base_addr);
-
-  descriptorBaseAddrMembers.push_back(createMapInfoOp(
-      firOpBuilder, loc, descDataBaseAddr, baseAddrAddr, asFortran, bounds, {},
-      static_cast<std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-          mapCaptureType),
-      mlir::omp::VariableCaptureKind::ByRef, descDataBaseAddr.getType()));
-
-  // TODO: map the addendum segment of the descriptor, similarly to the above
-  // base address/data pointer member.
-
-  return createMapInfoOp(
-      firOpBuilder, loc, descriptor, mlir::Value{}, asFortran, {},
-      descriptorBaseAddrMembers,
-      static_cast<std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-          mapCaptureType),
-      mlir::omp::VariableCaptureKind::ByRef, descriptor.getType());
-}
-
 bool ClauseProcessor::processMap(
     mlir::Location currentLocation, const llvm::omp::Directive &directive,
     Fortran::semantics::SemanticsContext &semanticsContext,
@@ -2008,32 +1962,21 @@ bool ClauseProcessor::processMap(
                   converter, firOpBuilder, semanticsContext, stmtCtx, ompObject,
                   clauseLocation, asFortran, bounds, treatIndexAsSection);
 
-          auto origSymbol =
-              converter.getSymbolAddress(*getOmpObjectSymbol(ompObject));
-          mlir::Value mapOp, symAddr;
-          if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType())) {
-            symAddr = origSymbol;
-            mapOp = processDescriptorTypeMappings(
-                semanticsContext, stmtCtx, converter, clauseLocation,
-                origSymbol, info.addr, bounds, asFortran.str(), mapTypeBits);
-          } else {
-            // Explicit map captures are captured ByRef by default,
-            // optimisation passes may alter this to ByCopy or other capture
-            // types to optimise
-            symAddr = info.addr;
-            mapOp = createMapInfoOp(
-                firOpBuilder, clauseLocation, info.addr, mlir::Value{},
-                asFortran.str(), bounds, {},
-                static_cast<std::underlying_type_t<
-                    llvm::omp::OpenMPOffloadMappingFlags>>(mapTypeBits),
-                mlir::omp::VariableCaptureKind::ByRef, info.addr.getType());
-          }
+          // Explicit map captures are captured ByRef by default,
+          // optimisation passes may alter this to ByCopy or other capture
+          // types to optimise
+          mlir::Value mapOp = createMapInfoOp(
+              firOpBuilder, clauseLocation, info.addr, asFortran, bounds,
+              static_cast<
+                  std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+                  mapTypeBits),
+              mlir::omp::VariableCaptureKind::ByRef, info.addr.getType());
 
           mapOperands.push_back(mapOp);
           if (mapSymTypes)
-            mapSymTypes->push_back(symAddr.getType());
+            mapSymTypes->push_back(info.addr.getType());
           if (mapSymLocs)
-            mapSymLocs->push_back(symAddr.getLoc());
+            mapSymLocs->push_back(info.addr.getLoc());
           if (mapSymbols)
             mapSymbols->push_back(getOmpObjectSymbol(ompObject));
         }
@@ -2127,55 +2070,40 @@ bool ClauseProcessor::processMotionClauses(
     Fortran::semantics::SemanticsContext &semanticsContext,
     Fortran::lower::StatementContext &stmtCtx,
     llvm::SmallVectorImpl<mlir::Value> &mapOperands) {
-  return findRepeatableClause<T>([&](const T *motionClause,
-                                     const Fortran::parser::CharBlock &source) {
-    mlir::Location clauseLocation = converter.genLocation(source);
-    fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  return findRepeatableClause<T>(
+      [&](const T *motionClause, const Fortran::parser::CharBlock &source) {
+        mlir::Location clauseLocation = converter.genLocation(source);
+        fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
 
-    static_assert(std::is_same_v<T, ClauseProcessor::ClauseTy::To> ||
-                  std::is_same_v<T, ClauseProcessor::ClauseTy::From>);
-
-    // TODO Support motion modifiers: present, mapper, iterator.
-    constexpr llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
-        std::is_same_v<T, ClauseProcessor::ClauseTy::To>
-            ? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO
-            : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
-
-    for (const Fortran::parser::OmpObject &ompObject : motionClause->v.v) {
-      llvm::SmallVector<mlir::Value> bounds;
-      std::stringstream asFortran;
-      Fortran::lower::AddrAndBoundsInfo info =
-          Fortran::lower::gatherDataOperandAddrAndBounds<
-              Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
-              mlir::omp::DataBoundsType>(
-              converter, firOpBuilder, semanticsContext, stmtCtx, ompObject,
-              clauseLocation, asFortran, bounds, treatIndexAsSection);
-
-      auto origSymbol =
-          converter.getSymbolAddress(*getOmpObjectSymbol(ompObject));
-      mlir::Value mapOp, symAddr;
-      if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType())) {
-        symAddr = origSymbol;
-        mapOp = processDescriptorTypeMappings(
-            semanticsContext, stmtCtx, converter, clauseLocation, origSymbol,
-            info.addr, bounds, asFortran.str(), mapTypeBits);
-      } else {
-        // Explicit map captures are captured ByRef by default,
-        // optimisation passes may alter this to ByCopy or other capture
-        // types to optimise
-        symAddr = info.addr;
-        mapOp = createMapInfoOp(
-            firOpBuilder, clauseLocation, info.addr, mlir::Value{},
-            asFortran.str(), bounds, {},
-            static_cast<
-                std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-                mapTypeBits),
-            mlir::omp::VariableCaptureKind::ByRef, info.addr.getType());
-      }
+        static_assert(std::is_same_v<T, ClauseProcessor::ClauseTy::To> ||
+                      std::is_same_v<T, ClauseProcessor::ClauseTy::From>);
 
-      mapOperands.push_back(mapOp);
-    }
-  });
+        // TODO Support motion modifiers: present, mapper, iterator.
+        constexpr llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
+            std::is_same_v<T, ClauseProcessor::ClauseTy::To>
+                ? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO
+                : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
+
+        for (const Fortran::parser::OmpObject &ompObject : motionClause->v.v) {
+          llvm::SmallVector<mlir::Value> bounds;
+          std::stringstream asFortran;
+          Fortran::lower::AddrAndBoundsInfo info =
+              Fortran::lower::gatherDataOperandAddrAndBounds<
+                  Fortran::parser::OmpObject, mlir::omp::DataBoundsOp,
+                  mlir::omp::DataBoundsType>(
+                  converter, firOpBuilder, semanticsContext, stmtCtx, ompObject,
+                  clauseLocation, asFortran, bounds, treatIndexAsSection);
+
+          mlir::Value mapOp = createMapInfoOp(
+              firOpBuilder, clauseLocation, info.addr, asFortran, bounds,
+              static_cast<
+                  std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+                  mapTypeBits),
+              mlir::omp::VariableCaptureKind::ByRef, info.addr.getType());
+
+          mapOperands.push_back(mapOp);
+        }
+      });
 }
 
 template <typename... Ts>
@@ -3136,8 +3064,7 @@ static void genBodyOfTargetOp(
         std::stringstream name;
         firOpBuilder.setInsertionPoint(targetOp);
         mlir::Value mapOp = createMapInfoOp(
-            firOpBuilder, copyVal.getLoc(), copyVal, mlir::Value{}, name.str(),
-            bounds, llvm::SmallVector<mlir::Value>{},
+            firOpBuilder, copyVal.getLoc(), copyVal, name, bounds,
             static_cast<
                 std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
                 llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_IMPLICIT),
@@ -3283,20 +3210,12 @@ genTargetOp(Fortran::lower::AbstractConverter &converter,
           mapFlag |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
         }
 
-        mlir::Value mapOp;
-        if (fir::isTypeWithDescriptor(baseOp.getType())) {
-          mapOp = processDescriptorTypeMappings(
-              semanticsContext, stmtCtx, converter, baseOp.getLoc(), baseOp,
-              info.addr, bounds, name.str(), mapFlag);
-        } else {
-          mapOp = createMapInfoOp(
-              converter.getFirOpBuilder(), baseOp.getLoc(), baseOp,
-              mlir::Value{}, name.str(), bounds, {},
-              static_cast<
-                  std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
-                  mapFlag),
-              captureKind, baseOp.getType());
-        }
+        mlir::Value mapOp = createMapInfoOp(
+            converter.getFirOpBuilder(), baseOp.getLoc(), baseOp, name, bounds,
+            static_cast<
+                std::underlying_type_t<llvm::omp::OpenMPOffloadMappingFlags>>(
+                mapFlag),
+            captureKind, baseOp.getType());
 
         mapOperands.push_back(mapOp);
         mapSymTypes.push_back(baseOp.getType());

flang/lib/Lower/OpenMP.cpp

@@ -1,5 +1,4 @@
 add_flang_library(FIRCodeGen
-  CodeGenOpenMP.cpp
   BoxedProcedure.cpp
   CGOps.cpp
   CodeGen.cpp

flang/lib/Optimizer/CodeGen/CMakeLists.txt

@@ -13,7 +13,6 @@
 #include "flang/Optimizer/CodeGen/CodeGen.h"
 
 #include "CGOps.h"
-#include "flang/Optimizer/CodeGen/CodeGenOpenMP.h"
 #include "flang/Optimizer/Dialect/FIRAttr.h"
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "flang/Optimizer/Dialect/FIRType.h"
@@ -3960,11 +3959,6 @@ class FIRToLLVMLowering
     mlir::populateMathToLibmConversionPatterns(pattern);
     mlir::populateComplexToLLVMConversionPatterns(typeConverter, pattern);
     mlir::populateVectorToLLVMConversionPatterns(typeConverter, pattern);
-
-    // Flang specific overloads for OpenMP operations, to allow for special
-    // handling of things like Box types.
-    fir::populateOpenMPFIRToLLVMConversionPatterns(typeConverter, pattern);
-
     mlir::ConversionTarget target{*context};
     target.addLegalDialect<mlir::LLVM::LLVMDialect>();
     // The OpenMP dialect is legal for Operations without regions, for those