[flang-commits] [flang] [flang] Code generation for fir.pack/unpack_array. (PR #132080)

[Slava Zakharin via flang-commits]

================
@@ -51,56 +49,59 @@ namespace fir {
 #define DEBUG_TYPE "lower-repack-arrays"
 
 namespace {
-class RepackArrayConversion {
-public:
-  RepackArrayConversion(std::optional<mlir::DataLayout> dataLayout)
-      : dataLayout(dataLayout) {}
-
-protected:
-  std::optional<mlir::DataLayout> dataLayout;
-
-  static bool canAllocateTempOnStack(mlir::Value box);
-};
-
-class PackArrayConversion : public mlir::OpRewritePattern<fir::PackArrayOp>,
-                            RepackArrayConversion {
+class PackArrayConversion : public mlir::OpRewritePattern<fir::PackArrayOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  PackArrayConversion(mlir::MLIRContext *context,
-                      std::optional<mlir::DataLayout> dataLayout)
-      : OpRewritePattern(context), RepackArrayConversion(dataLayout) {}
-
   mlir::LogicalResult
   matchAndRewrite(fir::PackArrayOp op,
                   mlir::PatternRewriter &rewriter) const override;
 
 private:
   static constexpr llvm::StringRef bufferName = ".repacked";
 
+  // Return value of fir::BaseBoxType that represents a temporary
+  // array created for the original box with given extents and
+  // type parameters. The new box has the default lower bounds.
+  // If useStack is true, then the temporary will be allocated
+  // in stack memory (when possible).
   static mlir::Value allocateTempBuffer(fir::FirOpBuilder &builder,
                                         mlir::Location loc, bool useStack,
                                         mlir::Value origBox,
                                         llvm::ArrayRef<mlir::Value> extents,
                                         llvm::ArrayRef<mlir::Value> typeParams);
+
+  // Generate value of fir::BaseBoxType that represents the result
+  // of the given fir.pack_array operation. The original box
+  // is assumed to be present (though, it may represent an empty array).
+  static mlir::FailureOr<mlir::Value> genRepackedBox(fir::FirOpBuilder &builder,
+                                                     mlir::Location loc,
+                                                     fir::PackArrayOp packOp);
 };
 
-class UnpackArrayConversion : public mlir::OpRewritePattern<fir::UnpackArrayOp>,
-                              RepackArrayConversion {
+class UnpackArrayConversion
+    : public mlir::OpRewritePattern<fir::UnpackArrayOp> {
 public:
   using OpRewritePattern::OpRewritePattern;
 
-  UnpackArrayConversion(mlir::MLIRContext *context,
-                        std::optional<mlir::DataLayout> dataLayout)
-      : OpRewritePattern(context), RepackArrayConversion(dataLayout) {}
-
   mlir::LogicalResult
   matchAndRewrite(fir::UnpackArrayOp op,
                   mlir::PatternRewriter &rewriter) const override;
 };
 } // anonymous namespace
 
-bool RepackArrayConversion::canAllocateTempOnStack(mlir::Value box) {
+// Return true iff for the given original boxed array we can
+// allocate temporary memory in stack memory.
+// This function is used to synchronize allocation/deallocation
+// implied by fir.pack_array and fir.unpack_array, because
+// the presence of the stack attribute does not automatically
+// mean that the allocation is actually done in stack memory.
+// For example, we always do the heap allocation for polymorphic
+// types using Fortran runtime.
+// Adding the polymorpic mold to fir.alloca and then using
+// Fortran runtime to compute the allocation size could probably
+// resolve this limitation.
+static bool canAllocateTempOnStack(mlir::Value box) {
----------------
vzakhari wrote:

Yes, I think putting it into the operation class is more appropriate.  I can make it a static member, because I need to use it for both `pack` and `unpack` coversions.

Thank you for the review, Tom!  I will merge this on Monday. I will be out for a couple of days and I do not want to break something and leave.

https://github.com/llvm/llvm-project/pull/132080