Skip to content

Conversation

@NimishMishra
Copy link
Contributor

Initialization of reduction variable for min-reduction is set to largest negative value. As such, in presence of non-negative operands, min reduction gives incorrect output. This patch initialises min-reduction to use the maximum positive value instead, so that it can produce correct output for the entire range of real valued operands.

Fixes #73101

@llvmbot
Copy link
Member

llvmbot commented Nov 22, 2023

@llvm/pr-subscribers-flang-openmp

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

Author: None (NimishMishra)

Changes

Initialization of reduction variable for min-reduction is set to largest negative value. As such, in presence of non-negative operands, min reduction gives incorrect output. This patch initialises min-reduction to use the maximum positive value instead, so that it can produce correct output for the entire range of real valued operands.

Fixes #73101


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

3 Files Affected:

  • (modified) flang/lib/Lower/OpenMP.cpp (+1-1)
  • (modified) flang/test/Lower/OpenMP/FIR/wsloop-reduction-min.f90 (+1-1)
  • (modified) flang/test/Lower/OpenMP/wsloop-reduction-min.f90 (+1-1)
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 517812305358c77..f6a61ba3a528e32 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -754,7 +754,7 @@ static mlir::Value getReductionInitValue(mlir::Location loc, mlir::Type type,
     if (auto ty = type.dyn_cast<mlir::FloatType>()) {
       const llvm::fltSemantics &sem = ty.getFloatSemantics();
       return builder.createRealConstant(
-          loc, type, llvm::APFloat::getSmallest(sem, /*Negative=*/true));
+          loc, type, llvm::APFloat::getLargest(sem, /*Negative=*/false));
     }
     unsigned bits = type.getIntOrFloatBitWidth();
     int64_t maxInt = llvm::APInt::getSignedMaxValue(bits).getSExtValue();
diff --git a/flang/test/Lower/OpenMP/FIR/wsloop-reduction-min.f90 b/flang/test/Lower/OpenMP/FIR/wsloop-reduction-min.f90
index 880d94b910af976..22cdd41c95179bd 100644
--- a/flang/test/Lower/OpenMP/FIR/wsloop-reduction-min.f90
+++ b/flang/test/Lower/OpenMP/FIR/wsloop-reduction-min.f90
@@ -2,7 +2,7 @@
 ! RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
 
 !CHECK: omp.reduction.declare @[[MIN_DECLARE_F:.*]] : f32 init {
-!CHECK:   %[[MAXIMUM_VAL_F:.*]] = arith.constant -1.401300e-45 : f32
+!CHECK:   %[[MAXIMUM_VAL_F:.*]] = arith.constant 3.40282347E+38 : f32
 !CHECK:   omp.yield(%[[MAXIMUM_VAL_F]] : f32)
 !CHECK: combiner
 !CHECK: ^bb0(%[[ARG0_F:.*]]: f32, %[[ARG1_F:.*]]: f32):
diff --git a/flang/test/Lower/OpenMP/wsloop-reduction-min.f90 b/flang/test/Lower/OpenMP/wsloop-reduction-min.f90
index c1c91122efddc63..af7f718b0b26d05 100644
--- a/flang/test/Lower/OpenMP/wsloop-reduction-min.f90
+++ b/flang/test/Lower/OpenMP/wsloop-reduction-min.f90
@@ -2,7 +2,7 @@
 ! RUN: %flang_fc1 -emit-hlfir -fopenmp -o - %s 2>&1 | FileCheck %s
 
 !CHECK: omp.reduction.declare @[[MIN_DECLARE_F:.*]] : f32 init {
-!CHECK:   %[[MAXIMUM_VAL_F:.*]] = arith.constant -1.401300e-45 : f32
+!CHECK:   %[[MAXIMUM_VAL_F:.*]] = arith.constant 3.40282347E+38 : f32
 !CHECK:   omp.yield(%[[MAXIMUM_VAL_F]] : f32)
 !CHECK: combiner
 !CHECK: ^bb0(%[[ARG0_F:.*]]: f32, %[[ARG1_F:.*]]: f32):

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @NimishMishra for the fix.

@NimishMishra NimishMishra merged commit 956cf0e into llvm:main Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[flang][OpenMP] reduction(min) gives an inexact negative value close to zero

3 participants