Skip to content

Commit 315c88c

Browse files
vzakharivzakhari
authored
[flang] Fixed MODULO(x, inf) to produce NaN. (#86145)
Straightforward computation of `A − FLOOR (A / P) * P` should produce NaN, when P is infinity. The -menable-no-infs lowering can still use the relaxed operations sequence.
1 parent 07a5667 commit 315c88c

File tree

5 files changed

+105
-15
lines changed

5 files changed

+105
-15
lines changed

‎flang/lib/Optimizer/Builder/IntrinsicCall.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5259,9 +5259,12 @@ mlir::Value IntrinsicLibrary::genModulo(mlir::Type resultType,
52595259
remainder);
52605260
}
52615261

5262+
auto fastMathFlags = builder.getFastMathFlags();
52625263
// F128 arith::RemFOp may be lowered to a runtime call that may be unsupported
52635264
// on the target, so generate a call to Fortran Runtime's ModuloReal16.
5264-
if (resultType == mlir::FloatType::getF128(builder.getContext()))
5265+
if (resultType == mlir::FloatType::getF128(builder.getContext()) ||
5266+
(fastMathFlags & mlir::arith::FastMathFlags::ninf) ==
5267+
mlir::arith::FastMathFlags::none)
52655268
return builder.createConvert(
52665269
loc, resultType,
52675270
fir::runtime::genModulo(builder, loc, args[0], args[1]));

‎flang/lib/Optimizer/Builder/Runtime/Numeric.cpp

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,20 @@ struct ForcedMod16 {
118118
}
119119
};
120120

121+
/// Placeholder for real*10 version of Modulo Intrinsic
122+
struct ForcedModulo10 {
123+
static constexpr const char *name = ExpandAndQuoteKey(RTNAME(ModuloReal10));
124+
static constexpr fir::runtime::FuncTypeBuilderFunc getTypeModel() {
125+
return [](mlir::MLIRContext *ctx) {
126+
auto fltTy = mlir::FloatType::getF80(ctx);
127+
auto strTy = fir::ReferenceType::get(mlir::IntegerType::get(ctx, 8));
128+
auto intTy = mlir::IntegerType::get(ctx, 8 * sizeof(int));
129+
return mlir::FunctionType::get(ctx, {fltTy, fltTy, strTy, intTy},
130+
{fltTy});
131+
};
132+
}
133+
};
134+
121135
/// Placeholder for real*16 version of Modulo Intrinsic
122136
struct ForcedModulo16 {
123137
static constexpr const char *name = ExpandAndQuoteKey(RTNAME(ModuloReal16));
@@ -349,7 +363,13 @@ mlir::Value fir::runtime::genModulo(fir::FirOpBuilder &builder,
349363

350364
// MODULO is lowered into math operations in intrinsics lowering,
351365
// so genModulo() should only be used for F128 data type now.
352-
if (fltTy.isF128())
366+
if (fltTy.isF32())
367+
func = fir::runtime::getRuntimeFunc<mkRTKey(ModuloReal4)>(loc, builder);
368+
else if (fltTy.isF64())
369+
func = fir::runtime::getRuntimeFunc<mkRTKey(ModuloReal8)>(loc, builder);
370+
else if (fltTy.isF80())
371+
func = fir::runtime::getRuntimeFunc<ForcedModulo10>(loc, builder);
372+
else if (fltTy.isF128())
353373
func = fir::runtime::getRuntimeFunc<ForcedModulo16>(loc, builder);
354374
else
355375
fir::intrinsicTypeTODO(builder, fltTy, loc, "MODULO");

‎flang/runtime/numeric-templates.h

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,8 +237,12 @@ inline RT_API_ATTRS T RealMod(
237237
if (ISNANTy<T>::compute(a) || ISNANTy<T>::compute(p) ||
238238
ISINFTy<T>::compute(a)) {
239239
return QNANTy<T>::compute();
240-
} else if (ISINFTy<T>::compute(p)) {
241-
return a;
240+
} else if (IS_MODULO && ISINFTy<T>::compute(p)) {
241+
// Other compilers behave consistently for MOD(x, +/-INF)
242+
// and always return x. This is probably related to
243+
// implementation of std::fmod(). Stick to this behavior
244+
// for MOD, but return NaN for MODULO(x, +/-INF).
245+
return QNANTy<T>::compute();
242246
}
243247
T aAbs{ABSTy<T>::compute(a)};
244248
T pAbs{ABSTy<T>::compute(p)};
@@ -248,8 +252,19 @@ inline RT_API_ATTRS T RealMod(
248252
if (auto pInt{static_cast<std::int64_t>(p)}; p == pInt) {
249253
// Fast exact case for integer operands
250254
auto mod{aInt - (aInt / pInt) * pInt};
251-
if (IS_MODULO && (aInt > 0) != (pInt > 0)) {
252-
mod += pInt;
255+
if constexpr (IS_MODULO) {
256+
if (mod == 0) {
257+
// Return properly signed zero.
258+
return pInt > 0 ? T{0} : -T{0};
259+
}
260+
if ((aInt > 0) != (pInt > 0)) {
261+
mod += pInt;
262+
}
263+
} else {
264+
if (mod == 0) {
265+
// Return properly signed zero.
266+
return aInt > 0 ? T{0} : -T{0};
267+
}
253268
}
254269
return static_cast<T>(mod);
255270
}
@@ -297,7 +312,11 @@ inline RT_API_ATTRS T RealMod(
297312
}
298313
if constexpr (IS_MODULO) {
299314
if ((a < 0) != (p < 0)) {
300-
tmp += p;
315+
if (tmp == 0.) {
316+
tmp = -tmp;
317+
} else {
318+
tmp += p;
319+
}
301320
}
302321
}
303322
return tmp;

‎flang/test/Lower/Intrinsics/modulo.f90

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
! RUN: bbc -emit-fir -hlfir=false %s -o - | FileCheck %s
1+
! RUN: bbc -emit-fir -hlfir=false %s -o - | FileCheck %s -check-prefixes=HONORINF,ALL
2+
! RUN: flang-new -fc1 -menable-no-infs -emit-fir -flang-deprecated-no-hlfir %s -o - | FileCheck %s -check-prefixes=CHECK,ALL
23

3-
! CHECK-LABEL: func @_QPmodulo_testr(
4-
! CHECK-SAME: %[[arg0:.*]]: !fir.ref<f64>{{.*}}, %[[arg1:.*]]: !fir.ref<f64>{{.*}}, %[[arg2:.*]]: !fir.ref<f64>{{.*}}) {
4+
! ALL-LABEL: func @_QPmodulo_testr(
5+
! ALL-SAME: %[[arg0:.*]]: !fir.ref<f64>{{.*}}, %[[arg1:.*]]: !fir.ref<f64>{{.*}}, %[[arg2:.*]]: !fir.ref<f64>{{.*}}) {
56
subroutine modulo_testr(r, a, p)
67
real(8) :: r, a, p
7-
! CHECK-DAG: %[[a:.*]] = fir.load %[[arg1]] : !fir.ref<f64>
8-
! CHECK-DAG: %[[p:.*]] = fir.load %[[arg2]] : !fir.ref<f64>
8+
! ALL-DAG: %[[a:.*]] = fir.load %[[arg1]] : !fir.ref<f64>
9+
! ALL-DAG: %[[p:.*]] = fir.load %[[arg2]] : !fir.ref<f64>
10+
! HONORINF: %[[res:.*]] = fir.call @_FortranAModuloReal8(%[[a]], %[[p]]
911
! CHECK-DAG: %[[rem:.*]] = arith.remf %[[a]], %[[p]] {{.*}}: f64
1012
! CHECK-DAG: %[[zero:.*]] = arith.constant 0.000000e+00 : f64
1113
! CHECK-DAG: %[[remNotZero:.*]] = arith.cmpf une, %[[rem]], %[[zero]] {{.*}} : f64
@@ -15,12 +17,12 @@ subroutine modulo_testr(r, a, p)
1517
! CHECK-DAG: %[[mustAddP:.*]] = arith.andi %[[remNotZero]], %[[signDifferent]] : i1
1618
! CHECK-DAG: %[[remPlusP:.*]] = arith.addf %[[rem]], %[[p]] {{.*}}: f64
1719
! CHECK: %[[res:.*]] = arith.select %[[mustAddP]], %[[remPlusP]], %[[rem]] : f64
18-
! CHECK: fir.store %[[res]] to %[[arg0]] : !fir.ref<f64>
20+
! ALL: fir.store %[[res]] to %[[arg0]] : !fir.ref<f64>
1921
r = modulo(a, p)
2022
end subroutine
2123

22-
! CHECK-LABEL: func @_QPmodulo_testi(
23-
! CHECK-SAME: %[[arg0:.*]]: !fir.ref<i64>{{.*}}, %[[arg1:.*]]: !fir.ref<i64>{{.*}}, %[[arg2:.*]]: !fir.ref<i64>{{.*}}) {
24+
! ALL-LABEL: func @_QPmodulo_testi(
25+
! ALL-SAME: %[[arg0:.*]]: !fir.ref<i64>{{.*}}, %[[arg1:.*]]: !fir.ref<i64>{{.*}}, %[[arg2:.*]]: !fir.ref<i64>{{.*}}) {
2426
subroutine modulo_testi(r, a, p)
2527
integer(8) :: r, a, p
2628
! CHECK-DAG: %[[a:.*]] = fir.load %[[arg1]] : !fir.ref<i64>

‎flang/unittests/Runtime/Numeric.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,30 @@ TEST(Numeric, Mod) {
6565
EXPECT_EQ(RTNAME(ModReal4)(Real<4>{-8.0}, Real<4>(5.0)), -3.0);
6666
EXPECT_EQ(RTNAME(ModReal8)(Real<8>{8.0}, Real<8>(-5.0)), 3.0);
6767
EXPECT_EQ(RTNAME(ModReal8)(Real<8>{-8.0}, Real<8>(-5.0)), -3.0);
68+
EXPECT_EQ(
69+
RTNAME(ModReal4)(Real<4>{0.5}, std::numeric_limits<Real<4>>::infinity()),
70+
0.5);
71+
EXPECT_EQ(
72+
RTNAME(ModReal4)(Real<4>{-0.5}, std::numeric_limits<Real<4>>::infinity()),
73+
-0.5);
74+
EXPECT_EQ(
75+
RTNAME(ModReal4)(Real<4>{0.5}, -std::numeric_limits<Real<4>>::infinity()),
76+
0.5);
77+
EXPECT_EQ(RTNAME(ModReal4)(
78+
Real<4>{-0.5}, -std::numeric_limits<Real<4>>::infinity()),
79+
-0.5);
80+
EXPECT_EQ(
81+
RTNAME(ModReal8)(Real<8>{0.5}, std::numeric_limits<Real<8>>::infinity()),
82+
0.5);
83+
EXPECT_EQ(
84+
RTNAME(ModReal8)(Real<8>{-0.5}, std::numeric_limits<Real<8>>::infinity()),
85+
-0.5);
86+
EXPECT_EQ(
87+
RTNAME(ModReal8)(Real<8>{0.5}, -std::numeric_limits<Real<8>>::infinity()),
88+
0.5);
89+
EXPECT_EQ(RTNAME(ModReal8)(
90+
Real<8>{-0.5}, -std::numeric_limits<Real<8>>::infinity()),
91+
-0.5);
6892
}
6993

7094
TEST(Numeric, Modulo) {
@@ -76,6 +100,28 @@ TEST(Numeric, Modulo) {
76100
EXPECT_EQ(RTNAME(ModuloReal4)(Real<4>{-8.0}, Real<4>(5.0)), 2.0);
77101
EXPECT_EQ(RTNAME(ModuloReal8)(Real<8>{8.0}, Real<8>(-5.0)), -2.0);
78102
EXPECT_EQ(RTNAME(ModuloReal8)(Real<8>{-8.0}, Real<8>(-5.0)), -3.0);
103+
// MODULO(x, INF) == NaN
104+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal4)(
105+
Real<4>{0.5}, std::numeric_limits<Real<4>>::infinity())));
106+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal4)(
107+
Real<4>{-0.5}, std::numeric_limits<Real<4>>::infinity())));
108+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal4)(
109+
Real<4>{0.5}, -std::numeric_limits<Real<4>>::infinity())));
110+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal4)(
111+
Real<4>{-0.5}, -std::numeric_limits<Real<4>>::infinity())));
112+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal8)(
113+
Real<8>{-0.5}, std::numeric_limits<Real<8>>::infinity())));
114+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal8)(
115+
Real<8>{0.5}, std::numeric_limits<Real<8>>::infinity())));
116+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal8)(
117+
Real<8>{-0.5}, -std::numeric_limits<Real<8>>::infinity())));
118+
EXPECT_TRUE(std::isnan(RTNAME(ModuloReal8)(
119+
Real<8>{0.5}, -std::numeric_limits<Real<8>>::infinity())));
120+
// MODULO(x, y) for integer values of x and y with 0 remainder.
121+
EXPECT_EQ(RTNAME(ModuloReal4)(Real<4>{5.0}, Real<4>(1.0)), 0.0);
122+
EXPECT_EQ(RTNAME(ModuloReal4)(Real<4>{5.0}, Real<4>(-1.0)), -0.0);
123+
EXPECT_EQ(RTNAME(ModuloReal4)(Real<4>{-5.0}, Real<4>(1.0)), 0.0);
124+
EXPECT_EQ(RTNAME(ModuloReal4)(Real<4>{-5.0}, Real<4>(-1.0)), -0.0);
79125
}
80126

81127
TEST(Numeric, Nearest) {

0 commit comments

Comments
 (0)