[Flang] Fix for Any/All simplification to properly propogate the inital value

[SBallantyne]

Summary

When rank > 1, the inital value would be lost on inner loops, leading to the wrong
value to be returned, e.g. This would return T. This patch fixes this to use the correct
inital value for all cases.

Integer :: m(0,10)
Any(m .eq 0)

[Leporacanthicus]

Aside from minor nit-picks, this looks good to me.

flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp

735

It's considered poor style [yes, I do this too, "do as I say, not as I do"] to introduce "other changes" with a bug-fix.

At the very least, it should be mentioned in the commit message as "Also removed some debug output code". It really doesn't hurt much to leave it in there, but don't undo this just because I said - more of "try to avoid in future".

flang/test/Transforms/simplifyintrinsics.fir

1470

Looks like you are capturing OK, but I don't see it being used later? Are those lines just removed [that's fine, but then you could just as well accept "anything" there?] - Same applies below.

[SBallantyne]

flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp

735

Fair enough, i understand that - these are just debug statements from testing that i missed before the patch went in - they shouldn't have been committed in the first place.

flang/test/Transforms/simplifyintrinsics.fir

1470

OK here is iterateVariable that determines if the next loop should go ahead, its assigned by the first value in the result statement on line 1480, the same way INIT is assigned a new value from the second value of that result statement.

[vdonaldson]

Looks ok to me

[vdonaldson]

And thanks for the fix!

flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp

genReductionLoop(fir::FirOpBuilder &builder, mlir::func::FuncOp &funcOp,
   // the 0 dimension.
   llvm::SmallVector<mlir::Value, 15> indices;
   for (unsigned i = rank; 0 < i; --i) {
     mlir::Value step = one;
     mlir::Value loopCount = bounds[i - 1];
     auto loop = builder.create<OP>(loc, zeroIdx, loopCount, step,
                                    unorderedOrInitialLoopCond,
                                    /*finalCountValue=*/false, init);
-    init = loop.getRegionIterArgs()[0];
+    init = loop.getRegionIterArgs()[resultIndex];
     indices.push_back(loop.getInductionVar());
     // Set insertion point to the loop body so that the next loop
     // is inserted inside the current one.
     builder.setInsertionPointToStart(loop.getBody());
   }
 
   // Reverse the indices such that they are ordered as:
   //   <dim-0-idx, dim-1-idx, ...>
void SimplifyIntrinsicsPass::simplifyLogicalDim0Reduction(
   unsigned rank = getDimCount(args[0]);
 
   // getDimCount returns a rank of 0 for assumed shape arrays, don't simplify in
   // these cases.
   if (!(isZero(dim) && rank > 0))
     return;
 
   mlir::Value inputBox = findBoxDef(args[0]);
-  LLVM_DEBUG(llvm::dbgs() << "Boxdef was: " << inputBox << '\n');
 
   mlir::Type elementType = hlfir::getFortranElementType(inputBox.getType());
   mlir::SymbolRefAttr callee = call.getCalleeAttr();
 
   fir::FirOpBuilder builder{getSimplificationBuilder(call, kindMap)};
 
-  LLVM_DEBUG(llvm::dbgs() << "In DIM0 simplify" << '\n');
   // Treating logicals as integers makes things a lot easier
   fir::LogicalType logicalType = {elementType.dyn_cast<fir::LogicalType>()};
-  LLVM_DEBUG(llvm::dbgs() << "Done logical cast, got: " << logicalType << '\n');
   fir::KindTy kind = logicalType.getFKind();
   mlir::Type intElementType =
       mlir::IntegerType::get(builder.getContext(), kind * 8);
 
   // Mangle kind into function name as it is not done by default
   std::string funcName =
       (mlir::Twine{callee.getLeafReference().getValue(), "Logical"} +
        mlir::Twine{kind} + "x" + mlir::Twine{rank})
           .str();
 
-  LLVM_DEBUG(llvm::dbgs() << "end of DIM0" << '\n');
-
   simplifyReductionBody(call, kindMap, genBodyFunc, builder, funcName,
                         intElementType);
 }
 
 void SimplifyIntrinsicsPass::simplifyLogicalDim1Reduction(
     fir::CallOp call, const fir::KindMapping &kindMap,
     GenReductionBodyTy genBodyFunc) {
 
if (auto call = mlir::dyn_cast<fir::CallOp>(op)) {
                      llvm::dbgs() << "\n");
           return;
         }
         if (funcName.startswith(RTNAME_STRING(Maxval))) {
           simplifyIntOrFloatReduction(call, kindMap, genRuntimeMaxvalBody);
           return;
         }
         if (funcName.startswith(RTNAME_STRING(Count))) {
-          LLVM_DEBUG(llvm::dbgs() << "Count" << '\n');
           simplifyLogicalDim0Reduction(call, kindMap, genRuntimeCountBody);
           return;
         }
         if (funcName.startswith(RTNAME_STRING(Any))) {
-          LLVM_DEBUG(llvm::dbgs() << "Any" << '\n');
           simplifyLogicalDim1Reduction(call, kindMap, genRuntimeAnyBody);
           return;
         }
         if (funcName.endswith(RTNAME_STRING(All))) {
           simplifyLogicalDim1Reduction(call, kindMap, genRuntimeAllBody);
           return;
         }
       }

flang/test/Transforms/simplifyintrinsics.fir

 
 // CHECK-LABEL:   func.func @_QPtestAny_UnknownDim(
 // CHECK-SAME:                          %[[VAL_0:.*]]: !fir.box<!fir.array<?x!fir.logical<4>>> {fir.bindc_name = "a"}) -> !fir.logical<4> {
 // CHECK-NOT        fir.call @_FortranAAnyLogical4x1_simplified({{.*}})
 // CHECK:           fir.call @_FortranAAny({{.*}}) fastmath<contract> : (!fir.box<none>, !fir.ref<i8>, i32, i32) -> i1
 // CHECK-NOT        fir.call @_FortranAAnyLogical4x1_simplified({{.*}})
 
 // -----
+// Check that multi-rank Any cases are properly simplified
+
+func.func @_QPtestAny_2D(%arg0: !fir.ref<!fir.array<10x0x!fir.logical<4>>> {fir.bindc_name = "a"}) -> !fir.logical<4> {
+    %c10 = arith.constant 10 : index
+    %c0 = arith.constant 0 : index
+    %0 = fir.alloca !fir.logical<4> {bindc_name = "testAny_2D", uniq_name = "_QFtestAny_2DEtestAny_2D"}
+    %1 = fir.shape %c10, %c0 : (index, index) -> !fir.shape<2>
+    %2 = fir.embox %arg0(%1) : (!fir.ref<!fir.array<10x0x!fir.logical<4>>>, !fir.shape<2>) -> !fir.box<!fir.array<10x0x!fir.logical<4>>>
+    %c1 = arith.constant 1 : index
+    %3 = fir.address_of(@_QQcl.04ab56883945fd2c21a3b6d132f0bb37) : !fir.ref<!fir.char<1,48>>
+    %c3_i32 = arith.constant 3 : i32
+    %4 = fir.convert %2 : (!fir.box<!fir.array<10x0x!fir.logical<4>>>) -> !fir.box<none>
+    %5 = fir.convert %3 : (!fir.ref<!fir.char<1,48>>) -> !fir.ref<i8>
+    %6 = fir.convert %c1 : (index) -> i32
+    %7 = fir.call @_FortranAAny(%4, %5, %c3_i32, %6) fastmath<contract> : (!fir.box<none>, !fir.ref<i8>, i32, i32) -> i1
+    %8 = fir.convert %7 : (i1) -> !fir.logical<4>
+    fir.store %8 to %0 : !fir.ref<!fir.logical<4>>
+    %9 = fir.load %0 : !fir.ref<!fir.logical<4>>
+    return %9 : !fir.logical<4>
+  }
+
+// CHECK-LABEL:   func.func @_QPtestAny_2D(
+// CHECK-SAME:                                        %[[A:.*]]: !fir.ref<!fir.array<10x0x!fir.logical<4>>> {fir.bindc_name = "a"}) -> !fir.logical<4> {
+// CHECK:           %[[SIZE10:.*]] = arith.constant 10 : index
+// CHECK:           %[[SIZE_0:.*]] = arith.constant 0 : index
+// CHECK:           %[[SHAPE:.*]] = fir.shape %[[SIZE10]], %[[SIZE_0]] : (index, index) -> !fir.shape<2>
+// CHECK:           %[[A_BOX_LOGICAL:.*]] = fir.embox %[[VAL_0]](%[[SHAPE]]) : (!fir.ref<!fir.array<10x0x!fir.logical<4>>>, !fir.shape<2>) -> !fir.box<!fir.array<10x0x!fir.logical<4>>>
+// CHECK:           %[[A_BOX_NONE:.*]] = fir.convert %[[A_BOX_LOGICAL]] : (!fir.box<!fir.array<10x0x!fir.logical<4>>>) -> !fir.box<none>
+// CHECK:           %[[RES:.*]] = fir.call @_FortranAAnyLogical4x2_simplified(%[[A_BOX_NONE]]) fastmath<contract> : (!fir.box<none>) -> i1
+
+// CHECK-LABEL:   func.func private @_FortranAAnyLogical4x2_simplified(
+// CHECK-SAME:                                                         %[[ARR:.*]]: !fir.box<none>) -> i1 attributes {llvm.linkage = #llvm.linkage<linkonce_odr>} {
+// CHECK:           %[[TRUE:.*]] = arith.constant true
+// CHECK:           %[[CINDEX_0:.*]] = arith.constant 0 : index
+// CHECK:           %[[A_BOX_I32:.*]] = fir.convert %[[VAL_0]] : (!fir.box<none>) -> !fir.box<!fir.array<?x?xi32>>
+// CHECK:           %[[FALSE:.*]] = arith.constant false
+// CHECK:           %[[CINDEX_1:.*]] = arith.constant 1 : index
+// CHECK:           %[[DIMINDEX_0:.*]] = arith.constant 0 : index
+// CHECK:           %[[DIMS:.*]]:3 = fir.box_dims %[[A_BOX_I32]], %[[DIMINDEX_0]] : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
+// CHECK:           %[[EXTENT:.*]] = arith.subi %[[DIMS]]#1, %[[CINDEX_1]] : index
+// CHECK:           %[[DIMINDEX_1:.*]] = arith.constant 1 : index
+// CHECK:           %[[DIMS_1:.*]]:3 = fir.box_dims %[[A_BOX_I32]], %[[DIMINDEX_1]] : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
+// CHECK:           %[[EXTENT1:.*]] = arith.subi %[[DIMS_1]]#1, %[[CINDEX_1]] : index
+// CHECK:           %[[RES:.*]]:2 = fir.iterate_while (%[[ITER:.*]] = %[[CINDEX_0]] to %[[EXTENT1]] step %[[CINDEX_1]]) and (%[[OK:.*]] = %[[TRUE]]) iter_args(%[[INIT:.*]] = %[[FALSE]]) -> (i1) {
+// CHECK:             %[[INNER_RES:.*]]:2 = fir.iterate_while (%[[ITER_1:.*]] = %[[CINDEX_0]] to %[[EXTENT]] step %[[CINDEX_1]]) and (%[[OK1:.*]] = %[[TRUE]]) iter_args(%[[INNER_INIT:.*]] = %[[INIT]]) -> (i1) {
+// CHECK:               %[[ITEM:.*]] = fir.coordinate_of %[[A_BOX_I32]], %[[ITER_1]], %[[ITER]] : (!fir.box<!fir.array<?x?xi32>>, index, index) -> !fir.ref<i32>
+// CHECK:               %[[ITEMVAL:.*]] = fir.load %[[ITEM]] : !fir.ref<i32>
+// CHECK:               %[[I32_0:.*]] = arith.constant 0 : i32
+// CHECK:               %[[NEXT_ANY:.*]] = arith.cmpi ne, %[[ITEMVAL]], %[[I32_0]] : i32
+// CHECK:               %[[I1_1:.*]] = arith.constant true
+// CHECK:               %[[CONTINUE:.*]] = arith.xori %[[NEXT_ANY]], %[[I1_1]] : i1
+// CHECK:               fir.result %[[CONTINUE]], %[[NEXT_ANY]] : i1, i1
+// CHECK:             }
+// CHECK:             fir.result %[[RETURN_VALS:.*]]#0, %[[RETURN_VALS]]#1 : i1, i1
+// CHECK:           }
+// CHECK:           return %[[RETURN_VAL:.*]]#1 : i1
+// CHECK:         }
+
+// -----
 // Ensure All is simplified in correct usage
 
 func.func @_QPtestAll_NoDimArg(%arg0: !fir.ref<!fir.array<10x!fir.logical<4>>> {fir.bindc_name = "a"}) -> !fir.logical<4> {
   %c10 = arith.constant 10 : index
   %0 = fir.alloca !fir.logical<4> {bindc_name = "testAll_NoDimArg", uniq_name = "_QFtestAll_NoDimArgEtestAll_NoDimArg"}
   %1 = fir.shape %c10 : (index) -> !fir.shape<1>
   %2 = fir.embox %arg0(%1) : (!fir.ref<!fir.array<10x!fir.logical<4>>>, !fir.shape<1>) -> !fir.box<!fir.array<10x!fir.logical<4>>>
   %c1 = arith.constant 1 : index

 // CHECK:             fir.result %[[CMP_AND_CONTINUE]], %[[CMP_AND_CONTINUE]] : i1, i1
 // CHECK:           }
 // CHECK:           return %[[RES:.*]]#1 : i1
 // CHECK:         }
 
 // -----
 // Ensure All is correctly simplified for different kind logical
 
-
 func.func @_QPtestAll_NoDimArgLogical1(%arg0: !fir.ref<!fir.array<10x!fir.logical<1>>> {fir.bindc_name = "a"}) -> !fir.logical<1> {
   %c10 = arith.constant 10 : index
   %0 = fir.alloca !fir.logical<1> {bindc_name = "testAll_NoDimArgLogical1", uniq_name = "_QFtestAll_NoDimArgLogical1EtestAll_NoDimArgLogical1"}
   %1 = fir.shape %c10 : (index) -> !fir.shape<1>
   %2 = fir.embox %arg0(%1) : (!fir.ref<!fir.array<10x!fir.logical<1>>>, !fir.shape<1>) -> !fir.box<!fir.array<10x!fir.logical<1>>>
   %c1 = arith.constant 1 : index
   %3 = fir.address_of(@_QQcl.04ab56883945fd2c21a3b6d132f0bb37) : !fir.ref<!fir.char<1,48>>
   %c3_i32 = arith.constant 3 : i32