[flang] probably convert Fortran logical to i1 in expanding hlfir.maxloc/hlfir.minloc opcodes

[kkwli]

Currently, if mask is a scalar, the generated code looks like:

    %22 = fir.convert %14 : (!fir.box<!fir.logical<4>>) -> !fir.box<none>
    fir.call @_FortranAMaxlocReal4x1_Logical4x0_i32_contract_simplified(%19, %20, %22) fastmath<contract> : (!fir.ref<!fir.box<none>>, !fir.box<none>, !fir.box<none>) -> ()
...
  func.func private @_FortranAMaxlocReal4x1_Logical4x0_i32_contract_simplified(%arg0: !fir.ref<!fir.box<none>>, %arg1: !fir.box<none>, %arg2: !fir.box<none>) attributes {llvm.linkage = #llvm.linkage<linkonce_odr>} {

    %5 = fir.convert %arg2 : (!fir.box<none>) -> !fir.box<!fir.array<1xi1>>
    %c0_0 = arith.constant 0 : index
    %6 = fir.coordinate_of %5, %c0_0 : (!fir.box<!fir.array<1xi1>>, index) -> !fir.ref<i1>
    %7 = fir.load %6 : !fir.ref<i1>

For the fir.convert from !fir.box<none> (i.e. !fir.box<!fir.logical<4>>) to !fir.box<!fir.array<1xi1>> it picks up the wrong value on the big endian platform. This patch is to do the conversion based on the original type of the mask and convert the value to i1 after the load.

Fix #110921

[llvmbot]

@llvm/pr-subscribers-flang-fir-hlfir

Author: Kelvin Li (kkwli)

Changes

Currently, if mask is a scalar, the generated code looks like:

    %22 = fir.convert %14 : (!fir.box<!fir.logical<4>>) -> !fir.box<none>
    fir.call @<!-- -->_FortranAMaxlocReal4x1_Logical4x0_i32_contract_simplified(%19, %20, %22) fastmath&lt;contract&gt; : (!fir.ref&lt;!fir.box&lt;none&gt;&gt;, !fir.box&lt;none&gt;, !fir.box&lt;none&gt;) -&gt; ()
...
  func.func private @<!-- -->_FortranAMaxlocReal4x1_Logical4x0_i32_contract_simplified(%arg0: !fir.ref&lt;!fir.box&lt;none&gt;&gt;, %arg1: !fir.box&lt;none&gt;, %arg2: !fir.box&lt;none&gt;) attributes {llvm.linkage = #llvm.linkage&lt;linkonce_odr&gt;} {

    %5 = fir.convert %arg2 : (!fir.box&lt;none&gt;) -&gt; !fir.box&lt;!fir.array&lt;1xi1&gt;&gt;
    %c0_0 = arith.constant 0 : index
    %6 = fir.coordinate_of %5, %c0_0 : (!fir.box&lt;!fir.array&lt;1xi1&gt;&gt;, index) -&gt; !fir.ref&lt;i1&gt;
    %7 = fir.load %6 : !fir.ref&lt;i1&gt;

For the fir.convert from !fir.box<none> (i.e. !fir.box<!fir.logical<4>>) to !fir.box<!fir.array<1xi1>> it picks up the wrong value on the big endian platform. This patch is to do the conversion based on the original type of the mask and convert the value to i1 after the load.

---

Full diff: https://github.com/llvm/llvm-project/pull/129791.diff

2 Files Affected:

• (modified) flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp (+4-2)
• (modified) flang/test/Transforms/simplifyintrinsics.fir (+8-6)

diff --git a/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp b/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
index df2887ff1422e..3baac62dd69f2 100644
--- a/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
+++ b/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
@@ -821,7 +821,8 @@ static void genRuntimeMinMaxlocBody(fir::FirOpBuilder &builder,
   // if mask is a logical scalar, we can check its value before the main loop
   // and either ignore the fact it is there or exit early.
   if (maskRank == 0) {
-    mlir::Type logical = builder.getI1Type();
+    mlir::Type i1Type = builder.getI1Type();
+    mlir::Type logical = maskElemType;
     mlir::IndexType idxTy = builder.getIndexType();
 
     fir::SequenceType::Shape singleElement(1, 1);
@@ -834,8 +835,9 @@ static void genRuntimeMinMaxlocBody(fir::FirOpBuilder &builder,
     mlir::Value condAddr =
         builder.create<fir::CoordinateOp>(loc, logicalRefTy, array, indx);
     mlir::Value cond = builder.create<fir::LoadOp>(loc, condAddr);
+    mlir::Value condI1 = builder.create<fir::ConvertOp>(loc, i1Type, cond);
 
-    fir::IfOp ifOp = builder.create<fir::IfOp>(loc, elementType, cond,
+    fir::IfOp ifOp = builder.create<fir::IfOp>(loc, elementType, condI1,
                                                /*withElseRegion=*/true);
 
     builder.setInsertionPointToStart(&ifOp.getElseRegion().front());
diff --git a/flang/test/Transforms/simplifyintrinsics.fir b/flang/test/Transforms/simplifyintrinsics.fir
index 9a95748c17258..1a66e077165b0 100644
--- a/flang/test/Transforms/simplifyintrinsics.fir
+++ b/flang/test/Transforms/simplifyintrinsics.fir
@@ -1996,10 +1996,11 @@ func.func @_QPtestminloc_works1d_scalarmask_f64(%arg0: !fir.ref<!fir.array<10xf6
 // CHECK:           %[[OUTARR_IDX0:.*]] = arith.constant 0 : index
 // CHECK:           %[[OUTARR_ITEM0:.*]] = fir.coordinate_of %[[BOX_OUTARR]], %[[OUTARR_IDX0]] : (!fir.box<!fir.heap<!fir.array<1xi32>>>, index) -> !fir.ref<i32>
 // CHECK:           fir.store %[[INIT_OUT_IDX]] to %[[OUTARR_ITEM0]] : !fir.ref<i32>
-// CHECK:           %[[BOX_MASK:.*]] = fir.convert %[[BOX_MASK_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<1xi1>>
+// CHECK:           %[[BOX_MASK:.*]] = fir.convert %[[BOX_MASK_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<1x!fir.logical<4>>>
 // CHECK:           %[[MASK_IDX0:.*]] = arith.constant 0 : index
-// CHECK:           %[[MASK_ITEM:.*]] = fir.coordinate_of %[[BOX_MASK]], %[[MASK_IDX0]] : (!fir.box<!fir.array<1xi1>>, index) -> !fir.ref<i1>
-// CHECK:           %[[MASK:.*]] = fir.load %[[MASK_ITEM]] : !fir.ref<i1>
+// CHECK:           %[[MASK_ITEM:.*]] = fir.coordinate_of %[[BOX_MASK]], %[[MASK_IDX0]] : (!fir.box<!fir.array<1x!fir.logical<4>>>, index) -> !fir.ref<!fir.logical<4>>
+// CHECK:           %[[MASK_LOGICAL:.*]] = fir.load %[[MASK_ITEM]] : !fir.ref<!fir.logical<4>>
+// CHECK:           %[[MASK:.*]] = fir.convert %[[MASK_LOGICAL]] : (!fir.logical<4>) -> i1
 // CHECK:           %[[INIT_RES:.*]] = fir.if %[[MASK]] -> (f64) {
 // CHECK:             %[[C_INDEX0:.*]] = arith.constant 0 : index
 // CHECK:             %[[BOX_INARR:.*]] = fir.convert %[[BOX_INARR_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<?xf64>>
@@ -2573,10 +2574,11 @@ func.func @_QPtestmaxloc_works1d_scalarmask_f64(%arg0: !fir.ref<!fir.array<10xf6
 // CHECK:           %[[OUTARR_IDX0:.*]] = arith.constant 0 : index
 // CHECK:           %[[OUTARR_ITEM0:.*]] = fir.coordinate_of %[[BOX_OUTARR]], %[[OUTARR_IDX0]] : (!fir.box<!fir.heap<!fir.array<1xi32>>>, index) -> !fir.ref<i32>
 // CHECK:           fir.store %[[INIT_OUT_IDX]] to %[[OUTARR_ITEM0]] : !fir.ref<i32>
-// CHECK:           %[[BOX_MASK:.*]] = fir.convert %[[BOX_MASK_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<1xi1>>
+// CHECK:           %[[BOX_MASK:.*]] = fir.convert %[[BOX_MASK_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<1x!fir.logical<4>>>
 // CHECK:           %[[MASK_IDX0:.*]] = arith.constant 0 : index
-// CHECK:           %[[MASK_ITEM:.*]] = fir.coordinate_of %[[BOX_MASK]], %[[MASK_IDX0]] : (!fir.box<!fir.array<1xi1>>, index) -> !fir.ref<i1>
-// CHECK:           %[[MASK:.*]] = fir.load %[[MASK_ITEM]] : !fir.ref<i1>
+// CHECK:           %[[MASK_ITEM:.*]] = fir.coordinate_of %[[BOX_MASK]], %[[MASK_IDX0]] : (!fir.box<!fir.array<1x!fir.logical<4>>>, index) -> !fir.ref<!fir.logical<4>>
+// CHECK:           %[[MASK_LOGICAL:.*]] = fir.load %[[MASK_ITEM]] : !fir.ref<!fir.logical<4>>
+// CHECK:           %[[MASK:.*]] = fir.convert %[[MASK_LOGICAL]] : (!fir.logical<4>) -> i1
 // CHECK:           %[[INIT_RES:.*]] = fir.if %[[MASK]] -> (f64) {
 // CHECK:             %[[C_INDEX0:.*]] = arith.constant 0 : index
 // CHECK:             %[[BOX_INARR:.*]] = fir.convert %[[BOX_INARR_NONE]] : (!fir.box<none>) -> !fir.box<!fir.array<?xf64>>

[vzakhari]

LGTM. Thank you for the fix!

[tblah]

LGTM, thanks!