[flang-commits] [flang] 03efa5a - [flang] Update the conversion code for fir.coordinate_of

Andrzej Warzynski via flang-commits flang-commits at lists.llvm.org
Mon Apr 4 03:15:23 PDT 2022


Author: Andrzej Warzynski
Date: 2022-04-04T10:15:14Z
New Revision: 03efa5a362a718b633a74cd9f1932dfa98f76a49

URL: https://github.com/llvm/llvm-project/commit/03efa5a362a718b633a74cd9f1932dfa98f76a49
DIFF: https://github.com/llvm/llvm-project/commit/03efa5a362a718b633a74cd9f1932dfa98f76a49.diff

LOG: [flang] Update the conversion code for fir.coordinate_of

These are mostly small changes to make the code a bit clearer and more
consistent. Summary of changes:
  * add missing namespace qualifiers (that's the preference in Flang)
  * replace const member methods with static methods (to avoid passing
    the *this pointer unnecessarily)
  * rename `currentObjTy` (current object type) as `cpnTy` (component
    type) - the latter feels more fitting
  * remove redundant `return failure();` calls (` return
    mlir::emitError` gives the same result)
  * updated a few comments

Differential Revision: https://reviews.llvm.org/D122799

Added: 
    

Modified: 
    flang/lib/Optimizer/CodeGen/CodeGen.cpp

Removed: 
    


################################################################################
diff  --git a/flang/lib/Optimizer/CodeGen/CodeGen.cpp b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
index 484edd9d30265..de8bc84e8feb5 100644
--- a/flang/lib/Optimizer/CodeGen/CodeGen.cpp
+++ b/flang/lib/Optimizer/CodeGen/CodeGen.cpp
@@ -2226,7 +2226,7 @@ struct CoordinateOpConversion
     if (fir::isa_complex(objectTy)) {
       mlir::LLVM::ConstantOp c0 =
           genConstantIndex(loc, lowerTy().indexType(), rewriter, 0);
-      SmallVector<mlir::Value> offs = {c0, operands[1]};
+      llvm::SmallVector<mlir::Value> offs = {c0, operands[1]};
       mlir::Value gep = genGEP(loc, ty, rewriter, base, offs);
       rewriter.replaceOp(coor, gep);
       return success();
@@ -2236,15 +2236,15 @@ struct CoordinateOpConversion
     if (baseObjectTy.dyn_cast<fir::BoxType>())
       return doRewriteBox(coor, ty, operands, loc, rewriter);
 
-    // Reference or pointer type
-    if (baseObjectTy.isa<fir::ReferenceType, fir::PointerType>())
+    // Reference, pointer or a heap type
+    if (baseObjectTy.isa<fir::ReferenceType, fir::PointerType, fir::HeapType>())
       return doRewriteRefOrPtr(coor, ty, operands, loc, rewriter);
 
     return rewriter.notifyMatchFailure(
         coor, "fir.coordinate_of base operand has unsupported type");
   }
 
-  unsigned getFieldNumber(fir::RecordType ty, mlir::Value op) const {
+  static unsigned getFieldNumber(fir::RecordType ty, mlir::Value op) {
     return fir::hasDynamicSize(ty)
                ? op.getDefiningOp()
                      ->getAttrOfType<mlir::IntegerAttr>("field")
@@ -2252,7 +2252,7 @@ struct CoordinateOpConversion
                : getIntValue(op);
   }
 
-  int64_t getIntValue(mlir::Value val) const {
+  static int64_t getIntValue(mlir::Value val) {
     assert(val && val.dyn_cast<mlir::OpResult>() && "must not be null value");
     mlir::Operation *defop = val.getDefiningOp();
 
@@ -2264,7 +2264,7 @@ struct CoordinateOpConversion
     fir::emitFatalError(val.getLoc(), "must be a constant");
   }
 
-  bool hasSubDimensions(mlir::Type type) const {
+  static bool hasSubDimensions(mlir::Type type) {
     return type.isa<fir::SequenceType, fir::RecordType, mlir::TupleType>();
   }
 
@@ -2273,7 +2273,7 @@ struct CoordinateOpConversion
   /// all valid forms of `!fir.coordinate_of`.
   /// TODO: Either implement the unsupported cases or extend the verifier
   /// in FIROps.cpp instead.
-  bool supportedCoordinate(mlir::Type type, mlir::ValueRange coors) const {
+  static bool supportedCoordinate(mlir::Type type, mlir::ValueRange coors) {
     const std::size_t numOfCoors = coors.size();
     std::size_t i = 0;
     bool subEle = false;
@@ -2302,10 +2302,8 @@ struct CoordinateOpConversion
   /// Walk the abstract memory layout and determine if the path traverses any
   /// array types with unknown shape. Return true iff all the array types have a
   /// constant shape along the path.
-  bool arraysHaveKnownShape(mlir::Type type, mlir::ValueRange coors) const {
-    const std::size_t sz = coors.size();
-    std::size_t i = 0;
-    for (; i < sz; ++i) {
+  static bool arraysHaveKnownShape(mlir::Type type, mlir::ValueRange coors) {
+    for (std::size_t i = 0, sz = coors.size(); i < sz; ++i) {
       mlir::Value nxtOpnd = coors[i];
       if (auto arrTy = type.dyn_cast<fir::SequenceType>()) {
         if (fir::sequenceWithNonConstantShape(arrTy))
@@ -2340,10 +2338,9 @@ struct CoordinateOpConversion
     if (coor.getNumOperands() == 2) {
       mlir::Operation *coordinateDef =
           (*coor.getCoor().begin()).getDefiningOp();
-      if (isa_and_nonnull<fir::LenParamIndexOp>(coordinateDef)) {
+      if (isa_and_nonnull<fir::LenParamIndexOp>(coordinateDef))
         TODO(loc,
              "fir.coordinate_of - fir.len_param_index is not supported yet");
-      }
     }
 
     // 2. GENERAL CASE:
@@ -2366,11 +2363,12 @@ struct CoordinateOpConversion
     mlir::Value resultAddr =
         loadBaseAddrFromBox(loc, getBaseAddrTypeFromBox(boxBaseAddr.getType()),
                             boxBaseAddr, rewriter);
-    auto currentObjTy = fir::dyn_cast_ptrOrBoxEleTy(boxObjTy);
+    // Component Type
+    auto cpnTy = fir::dyn_cast_ptrOrBoxEleTy(boxObjTy);
     mlir::Type voidPtrTy = ::getVoidPtrType(coor.getContext());
 
     for (unsigned i = 1, last = operands.size(); i < last; ++i) {
-      if (auto arrTy = currentObjTy.dyn_cast<fir::SequenceType>()) {
+      if (auto arrTy = cpnTy.dyn_cast<fir::SequenceType>()) {
         if (i != 1)
           TODO(loc, "fir.array nested inside other array and/or derived type");
         // Applies byte strides from the box. Ignore lower bound from box
@@ -2393,16 +2391,16 @@ struct CoordinateOpConversion
         resultAddr = rewriter.create<mlir::LLVM::GEPOp>(loc, voidPtrTy,
                                                         voidPtrBase, args);
         i += arrTy.getDimension() - 1;
-        currentObjTy = arrTy.getEleTy();
-      } else if (auto recTy = currentObjTy.dyn_cast<fir::RecordType>()) {
+        cpnTy = arrTy.getEleTy();
+      } else if (auto recTy = cpnTy.dyn_cast<fir::RecordType>()) {
         auto recRefTy =
             mlir::LLVM::LLVMPointerType::get(lowerTy().convertType(recTy));
         mlir::Value nxtOpnd = operands[i];
         auto memObj =
             rewriter.create<mlir::LLVM::BitcastOp>(loc, recRefTy, resultAddr);
         llvm::SmallVector<mlir::Value> args = {c0, nxtOpnd};
-        currentObjTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
-        auto llvmCurrentObjTy = lowerTy().convertType(currentObjTy);
+        cpnTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+        auto llvmCurrentObjTy = lowerTy().convertType(cpnTy);
         auto gep = rewriter.create<mlir::LLVM::GEPOp>(
             loc, mlir::LLVM::LLVMPointerType::get(llvmCurrentObjTy), memObj,
             args);
@@ -2423,25 +2421,25 @@ struct CoordinateOpConversion
                     mlir::ConversionPatternRewriter &rewriter) const {
     mlir::Type baseObjectTy = coor.getBaseType();
 
-    mlir::Type currentObjTy = fir::dyn_cast_ptrOrBoxEleTy(baseObjectTy);
-    bool hasSubdimension = hasSubDimensions(currentObjTy);
+    // Component Type
+    mlir::Type cpnTy = fir::dyn_cast_ptrOrBoxEleTy(baseObjectTy);
+    bool hasSubdimension = hasSubDimensions(cpnTy);
     bool columnIsDeferred = !hasSubdimension;
 
-    if (!supportedCoordinate(currentObjTy, operands.drop_front(1))) {
+    if (!supportedCoordinate(cpnTy, operands.drop_front(1)))
       TODO(loc, "unsupported combination of coordinate operands");
-    }
 
     const bool hasKnownShape =
-        arraysHaveKnownShape(currentObjTy, operands.drop_front(1));
+        arraysHaveKnownShape(cpnTy, operands.drop_front(1));
 
     // If only the column is `?`, then we can simply place the column value in
     // the 0-th GEP position.
-    if (auto arrTy = currentObjTy.dyn_cast<fir::SequenceType>()) {
+    if (auto arrTy = cpnTy.dyn_cast<fir::SequenceType>()) {
       if (!hasKnownShape) {
         const unsigned sz = arrTy.getDimension();
         if (arraysHaveKnownShape(arrTy.getEleTy(),
                                  operands.drop_front(1 + sz))) {
-          llvm::ArrayRef<int64_t> shape = arrTy.getShape();
+          fir::SequenceType::ShapeRef shape = arrTy.getShape();
           bool allConst = true;
           for (unsigned i = 0; i < sz - 1; ++i) {
             if (shape[i] < 0) {
@@ -2455,11 +2453,9 @@ struct CoordinateOpConversion
       }
     }
 
-    if (fir::hasDynamicSize(fir::unwrapSequenceType(currentObjTy))) {
-      mlir::emitError(
+    if (fir::hasDynamicSize(fir::unwrapSequenceType(cpnTy)))
+      return mlir::emitError(
           loc, "fir.coordinate_of with a dynamic element size is unsupported");
-      return failure();
-    }
 
     if (hasKnownShape || columnIsDeferred) {
       SmallVector<mlir::Value> offs;
@@ -2468,16 +2464,13 @@ struct CoordinateOpConversion
             genConstantIndex(loc, lowerTy().indexType(), rewriter, 0);
         offs.push_back(c0);
       }
-      const std::size_t sz = operands.size();
       Optional<int> dims;
       SmallVector<mlir::Value> arrIdx;
-      for (std::size_t i = 1; i < sz; ++i) {
+      for (std::size_t i = 1, sz = operands.size(); i < sz; ++i) {
         mlir::Value nxtOpnd = operands[i];
 
-        if (!currentObjTy) {
-          mlir::emitError(loc, "invalid coordinate/check failed");
-          return failure();
-        }
+        if (!cpnTy)
+          return mlir::emitError(loc, "invalid coordinate/check failed");
 
         // check if the i-th coordinate relates to an array
         if (dims.hasValue()) {
@@ -2487,32 +2480,32 @@ struct CoordinateOpConversion
             dims = dimsLeft - 1;
             continue;
           }
-          currentObjTy = currentObjTy.cast<fir::SequenceType>().getEleTy();
+          cpnTy = cpnTy.cast<fir::SequenceType>().getEleTy();
           // append array range in reverse (FIR arrays are column-major)
           offs.append(arrIdx.rbegin(), arrIdx.rend());
           arrIdx.clear();
           dims.reset();
           continue;
         }
-        if (auto arrTy = currentObjTy.dyn_cast<fir::SequenceType>()) {
+        if (auto arrTy = cpnTy.dyn_cast<fir::SequenceType>()) {
           int d = arrTy.getDimension() - 1;
           if (d > 0) {
             dims = d;
             arrIdx.push_back(nxtOpnd);
             continue;
           }
-          currentObjTy = currentObjTy.cast<fir::SequenceType>().getEleTy();
+          cpnTy = cpnTy.cast<fir::SequenceType>().getEleTy();
           offs.push_back(nxtOpnd);
           continue;
         }
 
         // check if the i-th coordinate relates to a field
-        if (auto recTy = currentObjTy.dyn_cast<fir::RecordType>())
-          currentObjTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
-        else if (auto tupTy = currentObjTy.dyn_cast<mlir::TupleType>())
-          currentObjTy = tupTy.getType(getIntValue(nxtOpnd));
+        if (auto recTy = cpnTy.dyn_cast<fir::RecordType>())
+          cpnTy = recTy.getType(getFieldNumber(recTy, nxtOpnd));
+        else if (auto tupTy = cpnTy.dyn_cast<mlir::TupleType>())
+          cpnTy = tupTy.getType(getIntValue(nxtOpnd));
         else
-          currentObjTy = nullptr;
+          cpnTy = nullptr;
 
         offs.push_back(nxtOpnd);
       }
@@ -2524,8 +2517,8 @@ struct CoordinateOpConversion
       return success();
     }
 
-    mlir::emitError(loc, "fir.coordinate_of base operand has unsupported type");
-    return failure();
+    return mlir::emitError(
+        loc, "fir.coordinate_of base operand has unsupported type");
   }
 };
 


        


More information about the flang-commits mailing list