[flang] Better error reporting for MOD/MODULO/NEAREST #96114
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When the second argument to these intrinsic functions is a scalar constant zero, emit a warning (if enabled) even if the first argument is not a constant.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesWhen the second argument to these intrinsic functions is a scalar constant zero, emit a warning (if enabled) even if the first argument is not a constant. Full diff: https://github.com/llvm/llvm-project/pull/96114.diff 4 Files Affected:
diff --git a/flang/lib/Evaluate/fold-integer.cpp b/flang/lib/Evaluate/fold-integer.cpp
index b76b9d49b5823..981cdff7f350b 100644
--- a/flang/lib/Evaluate/fold-integer.cpp
+++ b/flang/lib/Evaluate/fold-integer.cpp
@@ -1116,14 +1116,25 @@ Expr<Type<TypeCategory::Integer, KIND>> FoldIntrinsicFunction(
return FoldMaxvalMinval<T>(
context, std::move(funcRef), RelationalOperator::LT, T::Scalar::HUGE());
} else if (name == "mod") {
+ bool badPConst{false};
+ if (auto *pExpr{UnwrapExpr<Expr<T>>(args[1])}) {
+ *pExpr = Fold(context, std::move(*pExpr));
+ if (auto pConst{GetScalarConstantValue<T>(*pExpr)}; pConst &&
+ pConst->IsZero() &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say("MOD: P argument is zero"_warn_en_US);
+ badPConst = true;
+ }
+ }
return FoldElementalIntrinsic<T, T, T>(context, std::move(funcRef),
ScalarFuncWithContext<T, T, T>(
- [](FoldingContext &context, const Scalar<T> &x,
+ [badPConst](FoldingContext &context, const Scalar<T> &x,
const Scalar<T> &y) -> Scalar<T> {
auto quotRem{x.DivideSigned(y)};
if (context.languageFeatures().ShouldWarn(
common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
- if (quotRem.divisionByZero) {
+ if (!badPConst && quotRem.divisionByZero) {
context.messages().Say("mod() by zero"_warn_en_US);
} else if (quotRem.overflow) {
context.messages().Say("mod() folding overflowed"_warn_en_US);
@@ -1132,12 +1143,23 @@ Expr<Type<TypeCategory::Integer, KIND>> FoldIntrinsicFunction(
return quotRem.remainder;
}));
} else if (name == "modulo") {
+ bool badPConst{false};
+ if (auto *pExpr{UnwrapExpr<Expr<T>>(args[1])}) {
+ *pExpr = Fold(context, std::move(*pExpr));
+ if (auto pConst{GetScalarConstantValue<T>(*pExpr)}; pConst &&
+ pConst->IsZero() &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say("MODULO: P argument is zero"_warn_en_US);
+ badPConst = true;
+ }
+ }
return FoldElementalIntrinsic<T, T, T>(context, std::move(funcRef),
- ScalarFuncWithContext<T, T, T>([](FoldingContext &context,
+ ScalarFuncWithContext<T, T, T>([badPConst](FoldingContext &context,
const Scalar<T> &x,
const Scalar<T> &y) -> Scalar<T> {
auto result{x.MODULO(y)};
- if (result.overflow &&
+ if (!badPConst && result.overflow &&
context.languageFeatures().ShouldWarn(
common::UsageWarning::FoldingException)) {
context.messages().Say("modulo() folding overflowed"_warn_en_US);
diff --git a/flang/lib/Evaluate/fold-real.cpp b/flang/lib/Evaluate/fold-real.cpp
index f71addcc4094a..69c7a924cc1c3 100644
--- a/flang/lib/Evaluate/fold-real.cpp
+++ b/flang/lib/Evaluate/fold-real.cpp
@@ -303,41 +303,72 @@ Expr<Type<TypeCategory::Real, KIND>> FoldIntrinsicFunction(
context, std::move(funcRef), RelationalOperator::LT, T::Scalar::HUGE());
} else if (name == "mod") {
CHECK(args.size() == 2);
+ bool badPConst{false};
+ if (auto *pExpr{UnwrapExpr<Expr<T>>(args[1])}) {
+ *pExpr = Fold(context, std::move(*pExpr));
+ if (auto pConst{GetScalarConstantValue<T>(*pExpr)}; pConst &&
+ pConst->IsZero() &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say("MOD: P argument is zero"_warn_en_US);
+ badPConst = true;
+ }
+ }
return FoldElementalIntrinsic<T, T, T>(context, std::move(funcRef),
- ScalarFunc<T, T, T>(
- [&context](const Scalar<T> &x, const Scalar<T> &y) -> Scalar<T> {
- auto result{x.MOD(y)};
- if (result.flags.test(RealFlag::DivideByZero) &&
- context.languageFeatures().ShouldWarn(
- common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
- context.messages().Say(
- "second argument to MOD must not be zero"_warn_en_US);
- }
- return result.value;
- }));
+ ScalarFunc<T, T, T>([&context, badPConst](const Scalar<T> &x,
+ const Scalar<T> &y) -> Scalar<T> {
+ auto result{x.MOD(y)};
+ if (!badPConst && result.flags.test(RealFlag::DivideByZero) &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say(
+ "second argument to MOD must not be zero"_warn_en_US);
+ }
+ return result.value;
+ }));
} else if (name == "modulo") {
CHECK(args.size() == 2);
+ bool badPConst{false};
+ if (auto *pExpr{UnwrapExpr<Expr<T>>(args[1])}) {
+ *pExpr = Fold(context, std::move(*pExpr));
+ if (auto pConst{GetScalarConstantValue<T>(*pExpr)}; pConst &&
+ pConst->IsZero() &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say("MODULO: P argument is zero"_warn_en_US);
+ badPConst = true;
+ }
+ }
return FoldElementalIntrinsic<T, T, T>(context, std::move(funcRef),
- ScalarFunc<T, T, T>(
- [&context](const Scalar<T> &x, const Scalar<T> &y) -> Scalar<T> {
- auto result{x.MODULO(y)};
- if (result.flags.test(RealFlag::DivideByZero) &&
- context.languageFeatures().ShouldWarn(
- common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
- context.messages().Say(
- "second argument to MODULO must not be zero"_warn_en_US);
- }
- return result.value;
- }));
+ ScalarFunc<T, T, T>([&context, badPConst](const Scalar<T> &x,
+ const Scalar<T> &y) -> Scalar<T> {
+ auto result{x.MODULO(y)};
+ if (!badPConst && result.flags.test(RealFlag::DivideByZero) &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingAvoidsRuntimeCrash)) {
+ context.messages().Say(
+ "second argument to MODULO must not be zero"_warn_en_US);
+ }
+ return result.value;
+ }));
} else if (name == "nearest") {
- if (const auto *sExpr{UnwrapExpr<Expr<SomeReal>>(args[1])}) {
+ if (auto *sExpr{UnwrapExpr<Expr<SomeReal>>(args[1])}) {
+ *sExpr = Fold(context, std::move(*sExpr));
return common::visit(
[&](const auto &sVal) {
using TS = ResultType<decltype(sVal)>;
+ bool badSConst{false};
+ if (auto sConst{GetScalarConstantValue<TS>(sVal)}; sConst &&
+ sConst->IsZero() &&
+ context.languageFeatures().ShouldWarn(
+ common::UsageWarning::FoldingValueChecks)) {
+ context.messages().Say("NEAREST: S argument is zero"_warn_en_US);
+ badSConst = true;
+ }
return FoldElementalIntrinsic<T, T, TS>(context, std::move(funcRef),
ScalarFunc<T, T, TS>([&](const Scalar<T> &x,
const Scalar<TS> &s) -> Scalar<T> {
- if (s.IsZero() &&
+ if (!badSConst && s.IsZero() &&
context.languageFeatures().ShouldWarn(
common::UsageWarning::FoldingValueChecks)) {
context.messages().Say(
diff --git a/flang/test/Evaluate/fold-nearest.f90 b/flang/test/Evaluate/fold-nearest.f90
index bd8b020c392ac..a7366e6d75407 100644
--- a/flang/test/Evaluate/fold-nearest.f90
+++ b/flang/test/Evaluate/fold-nearest.f90
@@ -28,6 +28,12 @@ module m1
logical, parameter :: test_15 = nearest(negZero, 0.) == minSubnormal
logical, parameter :: test_16 = nearest(tiny(1.),-1.) == 1.1754942E-38
logical, parameter :: test_17 = nearest(tiny(1.),1.) == 1.1754945E-38
+ contains
+ subroutine subr(a)
+ real, intent(in) :: a
+ !WARN: warning: NEAREST: S argument is zero
+ print *, nearest(a, 0.)
+ end
end module
module m2
diff --git a/flang/test/Evaluate/folding04.f90 b/flang/test/Evaluate/folding04.f90
index 86ae8debd6ef1..c7815b0340360 100644
--- a/flang/test/Evaluate/folding04.f90
+++ b/flang/test/Evaluate/folding04.f90
@@ -32,11 +32,22 @@ module real_tests
!WARN: warning: invalid argument on evaluation of intrinsic function or operation
real(4), parameter :: nan_r4_acos5 = acos(r4_pinf)
TEST_ISNAN(nan_r4_acos5)
- !WARN: warning: second argument to MOD must not be zero
+ !WARN: warning: MOD: P argument is zero
real(4), parameter :: nan_r4_mod = mod(3.5, 0.)
TEST_ISNAN(nan_r4_mod)
!WARN: warning: overflow on evaluation of intrinsic function or operation
logical, parameter :: test_exp_overflow = exp(256._4).EQ.r4_pinf
+ contains
+ subroutine s1(a,j)
+ !WARN: warning: MOD: P argument is zero
+ print *, mod(a, 0.)
+ !WARN: warning: MODULO: P argument is zero
+ print *, modulo(a, 0.)
+ !WARN: warning: MOD: P argument is zero
+ print *, mod(j, 0.)
+ !WARN: warning: MODULO: P argument is zero
+ print *, modulo(j, 0.)
+ end
end module
module parentheses
|
psteinfeld
approved these changes
Jun 20, 2024
AlexisPerry
pushed a commit
to llvm-project-tlp/llvm-project
that referenced
this pull request
Jul 9, 2024
When the second argument to these intrinsic functions is a scalar constant zero, emit a warning (if enabled) even if the first argument is not a constant.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When the second argument to these intrinsic functions is a scalar constant zero, emit a warning (if enabled) even if the first argument is not a constant.