[Flang] Restricting dynamic arrays stack to heap move

[d-smirnov]

Summary

This patch restricts moving dynamic arrays to heap to arrays with all dynamic dimensions only
(i.e fir.array<?x?xf64> but not fir.array<5x?xf64>).
The patch also enables -fdynamic-heap-array flag by default.

Context: An issue was observed in 503.bwaves_r, where a function having an alloca with a dynamic size was not being inlined in LLVM. Converting to heap-arrays enables inlining.

[vzakhari]

Thank you for working on this.

Could you please provide more information about the LLVM behavior for the dynamically sized alloca? What sort of a heuristics prevents the inlining? Can you share optimization remarks (if there is any valuable info in there)?

flang/include/flang/Tools/CLOptions.inc

33

Please change the default value in a separate commit. I also think that such a change has to be accompanied with at least some performance data for the major benchmarks, e.g. a summary of regressions/improvements caused by this change would be useful.

flang/include/flang/Tools/CLOptions.inc

#define DisableOption(DOName, DOOption, DODescription) \
   static llvm::cl::opt<bool> disable##DOName("disable-" DOOption, \
       llvm::cl::desc("disable " DODescription " pass"), llvm::cl::init(false), \
       llvm::cl::Hidden)
 
 /// Shared option in tools to control whether dynamically sized array
 /// allocations should always be on the heap.
 static llvm::cl::opt<bool> dynamicArrayStackToHeapAllocation(
     "fdynamic-heap-array",
-    llvm::cl::desc("place all array allocations of dynamic size on the heap"),
-    llvm::cl::init(false), llvm::cl::Hidden);
+    llvm::cl::desc("place all array allocations of fully dynamic size on the heap"),
+    llvm::cl::init(true), llvm::cl::Hidden);
 
 /// Shared option in tools to set a maximum value for the number of elements in
 /// a compile-time sized array that can be allocated on the stack.
 static llvm::cl::opt<std::size_t> arrayStackAllocationThreshold(
     "fstack-array-size",
     llvm::cl::desc(
         "place all array allocations more than <size> elements on the heap"),
     llvm::cl::init(~static_cast<std::size_t>(0)), llvm::cl::Hidden);

flang/lib/Optimizer/Transforms/MemoryAllocation.cpp

static inline bool keepStackAllocation(fir::AllocaOp alloca, mlir::Block *entry,
   // Limitation: only arrays allocated on the stack in the entry block are
   // considered for now.
   // TODO: Generalize the algorithm and placement of the freemem nodes.
   if (alloca->getBlock() != entry)
     return true;
   if (auto seqTy = alloca.getInType().dyn_cast<fir::SequenceType>()) {
     if (fir::hasDynamicSize(seqTy)) {
       // Move all arrays with runtime determined size to the heap.
-      if (options.dynamicArrayOnHeap)
+      if (options.dynamicArrayOnHeap) {
+        for (const auto d : seqTy.getShape()) {
+          // Keep array on stack if some of the dimensions are static
+          if (d != seqTy.getUnknownExtent())
+            return true;
+        }
+        // move to heap otherwise
         return false;
+      }
     } else {
       std::int64_t numberOfElements = 1;
       for (std::int64_t i : seqTy.getShape()) {
         numberOfElements *= i;
         // If the count is suspicious, then don't change anything here.
         if (numberOfElements <= 0)
           return true;
       }

flang/test/Fir/alloc.fir

-// RUN: tco %s | FileCheck %s
-// RUN: %flang_fc1 -emit-llvm  %s -o - | FileCheck %s
+// RUN: %flang_fc1 -mllvm -fdynamic-heap-array=false -emit-llvm  %s -o - | FileCheck %s
 
 // UNSUPPORTED: system-windows
 
 // CHECK-LABEL: define ptr @alloca_scalar_nonchar()
 // CHECK: alloca i32, i64 1
 func.func @alloca_scalar_nonchar() -> !fir.ref<i32> {
   %1 = fir.alloca i32
   return %1 : !fir.ref<i32>

flang/test/Lower/array-stack.f90

+! Tests moving stack allocated dynamic arrays with all dynamic dimensions to heap
+! RUN: bbc %s -o - | FileCheck %s
+
+! CHECK-LABEL:  func @_QPss2(
+! CHECK: %[[A:.*]] = fir.alloca !fir.array<5xf32> {bindc_name = "a", uniq_name = "{{.*}}Ea"}
+! CHECK: %[[B:.*]] = fir.allocmem !fir.array<?xf32>, %4 {bindc_name = "b", uniq_name = "{{.*}}Eb"}
+! CHECK: %[[C:.*]] = fir.allocmem !fir.array<?x?xi32>, %4, %4 {bindc_name = "c", uniq_name = "{{.*}}Ec"}
+! CHECK: %[[D:.*]] = fir.alloca !fir.array<5x?x?xi32>, %4, %4 {bindc_name = "d", uniq_name = "{{.*}}Ed"}
+! CHECK: fir.freemem %[[B]] : !fir.heap<!fir.array<?xf32>>
+! CHECK: fir.freemem %[[C]] : !fir.heap<!fir.array<?x?xi32>>
+
+subroutine ss2(N)
+  real :: A(5)
+  real :: B(N)
+  integer :: C(N,N)
+  integer :: D(5,N,N)
+end

flang/test/Lower/array-temp.f90

-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc %s -fdynamic-heap-array=false -o - | FileCheck %s
 
 ! CHECK-LABEL: func @_QPss1()
 subroutine ss1
   ! CHECK: %[[aa:[0-9]+]] = fir.alloca !fir.array<2650000xf32> {bindc_name = "aa", uniq_name = "_QFss1Eaa"}
   ! CHECK: %[[shape:[0-9]+]] = fir.shape {{.*}} : (index) -> !fir.shape<1>
   integer, parameter :: N = 2650000
   real aa(N)
   ! CHECK: fir.array_coor %[[aa]](%[[shape]]) {{.*}} : (!fir.ref<!fir.array<2650000xf32>>, !fir.shape<1>, index) -> !fir.ref<f32>

flang/test/Lower/character-local-variables.f90

-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc %s -fdynamic-heap-array=false -o - | FileCheck %s
 
 ! Test lowering of local character variables
 
 ! CHECK-LABEL: func @_QPscalar_cst_len
 subroutine scalar_cst_len()
   character(10) :: c
   ! CHECK: fir.alloca !fir.char<1,10> {{{.*}}uniq_name = "_QFscalar_cst_lenEc"}
 end subroutine

flang/test/Lower/explicit-interface-results-2.f90

 ! Test lowering of internal procedures returning arrays or characters.
 ! This test allocation on the caller side of the results that may depend on
 ! host associated symbols.
-! RUN: bbc %s -o - | FileCheck %s
+! RUN: bbc %s -fdynamic-heap-array=false -o - | FileCheck %s
 
 module some_module
  integer :: n_module
 end module
 
 ! Test host calling array internal procedure.
 ! Result depends on host variable.
 ! CHECK-LABEL: func @_QPhost1