[flang-commits] [flang] [Flang] Skip SimplifyIntrinsics for unsigned reductions (PR #220850)

[via flang-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

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

Author: jay0x (blazie2004)

<details>
<summary>Changes</summary>

SimplifyIntrinsics assumes that the reduction argument element type matches the runtime call result type.

For UNSIGNED reductions, this is not true. For example, MAXVAL on an UNSIGNED(4) array is lowered with an array element type of ui32, while the runtime call returns i32 and is converted back to ui32 afterward.

This causes an assertions-enabled build to fail at -O1 and above with:

```
Assertion `*argType == resultType &&
           "Argument/result types mismatch in reduction"' failed.
```

The existing simplified integer reduction path is also not safe for unsigned values. In particular, MAXVAL currently uses a signed minimum value as the reduction identity and arith::MaxSIOp for the comparison.

Instead of weakening the assertion, skip SimplifyIntrinsics when the input element type is unsigned and preserve the existing runtime call.

This keeps the correct unsigned semantics while leaving signed integer and floating-point reductions unchanged.

fixes : [218926](https://github.com/llvm/llvm-project/issues/218926)

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


2 Files Affected:

- (modified) flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp (+6) 
- (modified) flang/test/Transforms/simplifyintrinsics.fir (+76) 


``````````diff
diff --git a/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp b/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
index 6319fa67f644b..6e1608c3c5877 100644
--- a/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
+++ b/flang/lib/Optimizer/Transforms/SimplifyIntrinsics.cpp
@@ -1058,6 +1058,12 @@ void SimplifyIntrinsicsPass::simplifyIntOrFloatReduction(
   auto argType = getArgElementType(args[0]);
   if (!argType)
     return;
+  // Unsigned reductions (e.g. MaxvalUnsigned/SumUnsigned) lower to runtime
+  // calls whose signless result type intentionally differs from the unsigned
+  // element type, and the inline reduction generated below would use a signed
+  // identity/comparison. Leave the correct runtime call in place.
+  if (argType->isUnsignedInteger())
+    return;
   assert(*argType == resultType &&
          "Argument/result types mismatch in reduction");
 
diff --git a/flang/test/Transforms/simplifyintrinsics.fir b/flang/test/Transforms/simplifyintrinsics.fir
index 2ef470f7f2ebd..b0542ee63b74a 100644
--- a/flang/test/Transforms/simplifyintrinsics.fir
+++ b/flang/test/Transforms/simplifyintrinsics.fir
@@ -849,6 +849,82 @@ module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.targ
 
 // -----
 
+// Call to MAXVAL with a 1D UNSIGNED array is not replaced: the runtime call
+// returns a signless integer that differs from the unsigned element type, so
+// the inline reduction (signed identity/comparison) would be incorrect. The
+// runtime call must be preserved.
+module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.target_triple = "native"} {
+  func.func @maxval_1d_array_unsigned(%arg0: !fir.ref<!fir.array<10xui32>> {fir.bindc_name = "a"}) -> ui32 {
+    %c10 = arith.constant 10 : index
+    %0 = fir.alloca ui32
+    %1 = fir.shape %c10 : (index) -> !fir.shape<1>
+    %2 = fir.embox %arg0(%1) : (!fir.ref<!fir.array<10xui32>>, !fir.shape<1>) -> !fir.box<!fir.array<10xui32>>
+    %3 = fir.absent !fir.box<i1>
+    %c0 = arith.constant 0 : index
+    %4 = fir.address_of(@_QQclXumax) : !fir.ref<!fir.char<1,13>>
+    %c5_i32 = arith.constant 5 : i32
+    %5 = fir.convert %2 : (!fir.box<!fir.array<10xui32>>) -> !fir.box<none>
+    %6 = fir.convert %4 : (!fir.ref<!fir.char<1,13>>) -> !fir.ref<i8>
+    %7 = fir.convert %c0 : (index) -> i32
+    %8 = fir.convert %3 : (!fir.box<i1>) -> !fir.box<none>
+    %9 = fir.call @_FortranAMaxvalUnsigned4(%5, %6, %c5_i32, %7, %8) : (!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32
+    %10 = fir.convert %9 : (i32) -> ui32
+    fir.store %10 to %0 : !fir.ref<ui32>
+    %11 = fir.load %0 : !fir.ref<ui32>
+    return %11 : ui32
+  }
+  func.func private @_FortranAMaxvalUnsigned4(!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32 attributes {fir.runtime}
+  fir.global linkonce @_QQclXumax constant : !fir.char<1,13> {
+    %0 = fir.string_lit "./umax_1.f90\00"(13) : !fir.char<1,13>
+    fir.has_value %0 : !fir.char<1,13>
+  }
+}
+
+// CHECK-LABEL:   func.func @maxval_1d_array_unsigned(
+// CHECK-NOT:       fir.call @_FortranAMaxvalUnsigned4x1_simplified({{.*}})
+// CHECK:           fir.call @_FortranAMaxvalUnsigned4({{.*}}) : (!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32
+// CHECK-NOT:       fir.call @_FortranAMaxvalUnsigned4x1_simplified({{.*}})
+
+// -----
+
+// Call to SUM with a 1D UNSIGNED array is not replaced: the runtime call
+// returns a signless integer that differs from the unsigned element type, so
+// the inline reduction (arith.addi on an unsigned value) would be invalid. The
+// runtime call must be preserved.
+module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.target_triple = "native"} {
+  func.func @sum_1d_array_unsigned(%arg0: !fir.ref<!fir.array<10xui32>> {fir.bindc_name = "a"}) -> ui32 {
+    %c10 = arith.constant 10 : index
+    %0 = fir.alloca ui32
+    %1 = fir.shape %c10 : (index) -> !fir.shape<1>
+    %2 = fir.embox %arg0(%1) : (!fir.ref<!fir.array<10xui32>>, !fir.shape<1>) -> !fir.box<!fir.array<10xui32>>
+    %3 = fir.absent !fir.box<i1>
+    %c0 = arith.constant 0 : index
+    %4 = fir.address_of(@_QQclXusum) : !fir.ref<!fir.char<1,13>>
+    %c5_i32 = arith.constant 5 : i32
+    %5 = fir.convert %2 : (!fir.box<!fir.array<10xui32>>) -> !fir.box<none>
+    %6 = fir.convert %4 : (!fir.ref<!fir.char<1,13>>) -> !fir.ref<i8>
+    %7 = fir.convert %c0 : (index) -> i32
+    %8 = fir.convert %3 : (!fir.box<i1>) -> !fir.box<none>
+    %9 = fir.call @_FortranASumUnsigned4(%5, %6, %c5_i32, %7, %8) : (!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32
+    %10 = fir.convert %9 : (i32) -> ui32
+    fir.store %10 to %0 : !fir.ref<ui32>
+    %11 = fir.load %0 : !fir.ref<ui32>
+    return %11 : ui32
+  }
+  func.func private @_FortranASumUnsigned4(!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32 attributes {fir.runtime}
+  fir.global linkonce @_QQclXusum constant : !fir.char<1,13> {
+    %0 = fir.string_lit "./usum_1.f90\00"(13) : !fir.char<1,13>
+    fir.has_value %0 : !fir.char<1,13>
+  }
+}
+
+// CHECK-LABEL:   func.func @sum_1d_array_unsigned(
+// CHECK-NOT:       fir.call @_FortranASumUnsigned4x1_simplified({{.*}})
+// CHECK:           fir.call @_FortranASumUnsigned4({{.*}}) : (!fir.box<none>, !fir.ref<i8>, i32, i32, !fir.box<none>) -> i32
+// CHECK-NOT:       fir.call @_FortranASumUnsigned4x1_simplified({{.*}})
+
+// -----
+
 // Call to MAXVAL with 1D F64 is replaced.
 module attributes {fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.target_triple = "native"} {
   func.func @maxval_1d_real(%arg0: !fir.ref<!fir.array<10xf64>> {fir.bindc_name = "a"}) -> f64 {

``````````

</details>


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