[flang-commits] [flang] [flang] Added LoopInvariantCodeMotion pass for [HL]FIR. (PR #173438)

[Slava Zakharin via flang-commits]

================
@@ -0,0 +1,233 @@
+//===- LoopInvariantCodeMotion.cpp ----------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+/// \file
+/// FIR-specific Loop Invariant Code Motion pass.
+/// The pass relies on FIR types and interfaces to prove the safety
+/// of hoisting invariant operations out of loop-like operations.
+/// It may be run on both HLFIR and FIR representations.
+//===----------------------------------------------------------------------===//
+
+#include "flang/Optimizer/Analysis/AliasAnalysis.h"
+#include "flang/Optimizer/Dialect/FortranVariableInterface.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/LoopInvariantCodeMotionUtils.h"
+#include "llvm/Support/DebugLog.h"
+
+namespace fir {
+#define GEN_PASS_DEF_LOOPINVARIANTCODEMOTION
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+#define DEBUG_TYPE "flang-licm"
+
+// Temporary engineering option for triaging LICM.
+static llvm::cl::opt<bool> disableFlangLICM(
+    "disable-flang-licm", llvm::cl::init(false), llvm::cl::Hidden,
+    llvm::cl::desc("Disable Flang's loop invariant code motion"));
+
+namespace {
+
+using namespace mlir;
+
+/// The pass tries to hoist loop invariant operations with only
+/// MemoryEffects::Read effects (MemoryEffects::Write support
+/// may be added later).
+/// The safety of hoisting is proven by:
+///   * Proving that the loop runs at least one iteration.
+///   * Proving that is is always safe to load from this location
+///     (see isSafeToHoistLoad() comments below).
+struct LoopInvariantCodeMotion
+    : fir::impl::LoopInvariantCodeMotionBase<LoopInvariantCodeMotion> {
+  void runOnOperation() override;
+};
+
+} // namespace
+
+/// 'location' is a memory reference used by a memory access.
+/// The type of 'location' defines the data type of the access
+/// (e.g. it is considered to be invalid to access 'i64'
+/// data using '!fir.ref<i32>`).
+/// For the given location, this function returns true iff
+/// the Fortran object being accessed is a scalar that
+/// may not be OPTIONAL.
+///
+/// Note that the '!fir.ref<!fir.box<>>' accesses are considered
+/// to be scalar, even if the underlying data is an array.
+///
+/// Note that an access of '!fir.ref<scalar>' may access
+/// an array object. For example:
+///   real :: x(:)
+///   do i=...
+///     = x(10)
+/// 'x(10)' accesses array 'x', and it may be unsafe to hoist
+/// it without proving that '10' is a valid index for the array.
+/// The fact that 'x' is not OPTIONAL does not allow hoisting
+/// on its own.
+static bool isNonOptionalScalar(Value location) {
----------------
vzakhari wrote:

For the `associate` test we have this currently:
```
func.func @_QPtest_associated_array_access(%arg0: !fir.ref<!fir.array<?xi32>> {fir.bindc_name = "a"}, %arg1: !fir.ref<!fir.array<?xi32>> {fir.bindc_name = "b"}, %arg2: !fir.ref<i32> {fir.bindc_name = "n"}) attributes {no_inline} {
  %c10 = arith.constant 10 : index
  %c1 = arith.constant 1 : index
  %0 = fir.dummy_scope : !fir.dscope
  %1 = fir.assumed_size_extent : index
  %2 = fir.shape %1 : (index) -> !fir.shape<1>
  %3 = fir.declare %arg0(%2) dummy_scope %0 arg 1 {uniq_name = "_QFtest_associated_array_accessEa"} : (!fir.ref<!fir.array<?xi32>>, !fir.shape<1>, !fir.dscope) -> !fir.ref<!fir.array<?xi32>>
  %4 = fir.declare %arg1(%2) dummy_scope %0 arg 2 {uniq_name = "_QFtest_associated_array_accessEb"} : (!fir.ref<!fir.array<?xi32>>, !fir.shape<1>, !fir.dscope) -> !fir.ref<!fir.array<?xi32>>
  %5 = fir.alloca i32 {bindc_name = "i", uniq_name = "_QFtest_associated_array_accessEi"}
  %6 = fir.declare %5 {uniq_name = "_QFtest_associated_array_accessEi"} : (!fir.ref<i32>) -> !fir.ref<i32>
  %7 = fir.declare %arg2 dummy_scope %0 arg 3 {uniq_name = "_QFtest_associated_array_accessEn"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32>
  %8 = fir.load %7 : !fir.ref<i32>
  %9 = fir.convert %8 : (i32) -> index
  %10 = fir.convert %c1 : (index) -> i32
  %11 = fir.do_loop %arg3 = %c1 to %9 step %c1 iter_args(%arg4 = %10) -> (i32) {
    fir.store %arg4 to %6 : !fir.ref<i32>
    // !!!
    %12 = fir.array_coor %4(%2) %c10 : (!fir.ref<!fir.array<?xi32>>, !fir.shape<1>, index) -> !fir.ref<i32>
    %13 = fir.declare %12 {uniq_name = "_QFtest_associated_array_accessEc"} : (!fir.ref<i32>) -> !fir.ref<i32>
    // !!!
    %14 = fir.load %13 : !fir.ref<i32>
    %15 = fir.load %6 : !fir.ref<i32>
    %16 = fir.convert %15 : (i32) -> i64
    %17 = fir.array_coor %3(%2) %16 : (!fir.ref<!fir.array<?xi32>>, !fir.shape<1>, i64) -> !fir.ref<i32>
    fir.store %14 to %17 : !fir.ref<i32>
    %18 = fir.load %6 : !fir.ref<i32>
    %19 = arith.addi %18, %10 overflow<nsw> : i32
    fir.result %19 : i32
  }
  fir.store %11 to %6 : !fir.ref<i32>
  return
}
```

I assume it should be safe to always hoist `%12 = fir.array_coor`. I am not sure about `%13 = fir.declare`, but I guess it should be also safe to hoist it (at least when we do not need precise debug information).

If I hoist both of them manually, then `%14 = fir.load` is hoisted by LICM, which is incorrect.  I will see if I can rely on the storage to still hoist the loads without the trip-count check.

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