[flang-commits] [flang] [flang] Improve warnings for invalid arguments when folding host runtime (PR #96807)
Pete Steinfeld via flang-commits
flang-commits at lists.llvm.org
Wed Jun 26 12:57:37 PDT 2024
https://github.com/psteinfeld updated https://github.com/llvm/llvm-project/pull/96807
>From 01d13405a698fb506110b888714994b1655f821f Mon Sep 17 00:00:00 2001
From: Peter Steinfeld <psteinfeld at nvidia.com>
Date: Wed, 26 Jun 2024 11:07:56 -0700
Subject: [PATCH 1/2] [flang] Improve warnings for invalid arguments when
folding host runtime
This is another attempt at a change that Jean made to Phabricator in
https://reviews.llvm.org/D116934. That attempt ran into problems with
building on Windows.
This change improves the messages that are produced when running into
invalid arguments during constant folding.
---
flang/lib/Evaluate/intrinsics-library.cpp | 240 +++++++++++++++++++++-
flang/test/Evaluate/folding04.f90 | 31 ++-
2 files changed, 263 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index 7315a7a057b10..e6588ba4ec9c1 100644
--- a/flang/lib/Evaluate/intrinsics-library.cpp
+++ b/flang/lib/Evaluate/intrinsics-library.cpp
@@ -633,10 +633,243 @@ static DynamicType BiggerType(DynamicType type) {
return type;
}
+/// Structure to register intrinsic argument checks that must be performed.
+using ArgumentVerifierFunc = bool (*)(
+ const std::vector<Expr<SomeType>> &, FoldingContext &);
+struct ArgumentVerifier {
+ using Key = std::string_view;
+ // Needed for implicit compare with keys.
+ constexpr operator Key() const { return key; }
+ Key key; // intrinsic name
+ ArgumentVerifierFunc verifier;
+};
+
+static constexpr int lastArg{-1};
+static constexpr int firstArg{0};
+
+static const Expr<SomeType> &getArg(
+ int position, const std::vector<Expr<SomeType>> &args) {
+ if (position == lastArg) {
+ CHECK(!args.empty());
+ return args.back();
+ }
+ CHECK(position >= 0 && static_cast<std::size_t>(position) < args.size());
+ return args[position];
+}
+
+template <typename T>
+static bool IsInRange(const Expr<T> &expr, int lb, int ub) {
+ if (auto scalar{GetScalarConstantValue<T>(expr)}) {
+ auto lbValue{Scalar<T>::FromInteger(value::Integer<8>{lb}).value};
+ auto ubValue{Scalar<T>::FromInteger(value::Integer<8>{ub}).value};
+ return Satisfies(RelationalOperator::LE, lbValue.Compare(*scalar)) &&
+ Satisfies(RelationalOperator::LE, scalar->Compare(ubValue));
+ }
+ return true;
+}
+
+/// Verify that the argument in an intrinsic call belongs to [lb, ub] if is
+/// real.
+template <int lb, int ub>
+static bool VerifyInRangeIfReal(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someReal =
+ std::get_if<Expr<SomeReal>>(&getArg(firstArg, args).u)) {
+ const bool isInRange{
+ std::visit([&](const auto &x) -> bool { return IsInRange(x, lb, ub); },
+ someReal->u)};
+ if (!isInRange) {
+ context.messages().Say(
+ "argument is out of range [%d., %d.]"_warn_en_US, lb, ub);
+ }
+ return isInRange;
+ }
+ return true;
+}
+
+template <int argPosition, const char *argName>
+static bool VerifyStrictlyPositiveIfReal(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someReal =
+ std::get_if<Expr<SomeReal>>(&getArg(argPosition, args).u)) {
+ const bool isStrictlyPositive{std::visit(
+ [&](const auto &x) -> bool {
+ using T = typename std::decay_t<decltype(x)>::Result;
+ auto scalar{GetScalarConstantValue<T>(x)};
+ return Satisfies(
+ RelationalOperator::LT, Scalar<T>{}.Compare(*scalar));
+ },
+ someReal->u)};
+ if (!isStrictlyPositive) {
+ context.messages().Say(
+ "argument '%s' must be strictly positive"_warn_en_US, argName);
+ }
+ return isStrictlyPositive;
+ }
+ return true;
+}
+
+/// Verify that an intrinsic call argument is not zero if it is real.
+template <int argPosition, const char *argName>
+static bool VerifyNotZeroIfReal(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someReal =
+ std::get_if<Expr<SomeReal>>(&getArg(argPosition, args).u)) {
+ const bool isNotZero{std::visit(
+ [&](const auto &x) -> bool {
+ using T = typename std::decay_t<decltype(x)>::Result;
+ auto scalar{GetScalarConstantValue<T>(x)};
+ return !scalar || !scalar->IsZero();
+ },
+ someReal->u)};
+ if (!isNotZero) {
+ context.messages().Say(
+ "argument '%s' must be different from zero"_warn_en_US, argName);
+ }
+ return isNotZero;
+ }
+ return true;
+}
+
+/// Verify that the argument in an intrinsic call is not zero if is complex.
+static bool VerifyNotZeroIfComplex(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someComplex =
+ std::get_if<Expr<SomeComplex>>(&getArg(firstArg, args).u)) {
+ const bool isNotZero{std::visit(
+ [&](const auto &z) -> bool {
+ using T = typename std::decay_t<decltype(z)>::Result;
+ auto scalar{GetScalarConstantValue<T>(z)};
+ return !scalar || !scalar->IsZero();
+ },
+ someComplex->u)};
+ if (!isNotZero) {
+ context.messages().Say(
+ "complex argument must be different from zero"_warn_en_US);
+ }
+ return isNotZero;
+ }
+ return true;
+}
+
+// Verify that the argument in an intrinsic call is not zero and not a negative
+// integer.
+static bool VerifyGammaLikeArgument(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someReal =
+ std::get_if<Expr<SomeReal>>(&getArg(firstArg, args).u)) {
+ const bool isValid{std::visit(
+ [&](const auto &x) -> bool {
+ using T = typename std::decay_t<decltype(x)>::Result;
+ auto scalar{GetScalarConstantValue<T>(x)};
+ if (scalar) {
+ return !scalar->IsZero() &&
+ !(scalar->IsNegative() &&
+ scalar->ToWholeNumber().value == scalar);
+ }
+ return true;
+ },
+ someReal->u)};
+ if (!isValid) {
+ context.messages().Say(
+ "argument must not be a negative integer or zero"_en_US);
+ }
+ return isValid;
+ }
+ return true;
+}
+
+// Verify that two real arguments are not both zero.
+static bool VerifyAtan2LikeArguments(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ if (const auto *someReal =
+ std::get_if<Expr<SomeReal>>(&getArg(firstArg, args).u)) {
+ const bool isValid{std::visit(
+ [&](const auto &typedExpr) -> bool {
+ using T = typename std::decay_t<decltype(typedExpr)>::Result;
+ auto x{GetScalarConstantValue<T>(typedExpr)};
+ auto y{GetScalarConstantValue<T>(getArg(lastArg, args))};
+ if (x && y) {
+ return !(x->IsZero() && y->IsZero());
+ }
+ return true;
+ },
+ someReal->u)};
+ if (!isValid) {
+ context.messages().Say(
+ "'x' and 'y' arguments must not be both zero"_en_US);
+ }
+ return isValid;
+ }
+ return true;
+}
+
+template <ArgumentVerifierFunc... F>
+static bool CombineVerifiers(
+ const std::vector<Expr<SomeType>> &args, FoldingContext &context) {
+ return (... & F(args, context));
+}
+
+/// Define argument names to be used error messages when the intrinsic have
+/// several arguments.
+static constexpr char xName[]{"x"};
+static constexpr char pName[]{"p"};
+
+/// Register argument verifiers for all intrinsics folded with runtime.
+static constexpr ArgumentVerifier intrinsicArgumentVerifiers[]{
+ {"acos", VerifyInRangeIfReal<-1, 1>},
+ {"asin", VerifyInRangeIfReal<-1, 1>},
+ {"atan2", VerifyAtan2LikeArguments},
+ {"bessel_y0", VerifyStrictlyPositiveIfReal<firstArg, xName>},
+ {"bessel_y1", VerifyStrictlyPositiveIfReal<firstArg, xName>},
+ {"bessel_yn", VerifyStrictlyPositiveIfReal<lastArg, xName>},
+ {"gamma", VerifyGammaLikeArgument},
+ {"log",
+ CombineVerifiers<VerifyStrictlyPositiveIfReal<firstArg, xName>,
+ VerifyNotZeroIfComplex>},
+ {"log10", VerifyStrictlyPositiveIfReal<firstArg, xName>},
+ {"log_gamma", VerifyGammaLikeArgument},
+ {"mod", VerifyNotZeroIfReal<lastArg, pName>},
+};
+
+const ArgumentVerifierFunc *findVerifier(const std::string &intrinsicName) {
+ static constexpr Fortran::common::StaticMultimapView<ArgumentVerifier>
+ verifiers(intrinsicArgumentVerifiers);
+ static_assert(verifiers.Verify(), "map must be sorted");
+ auto range{verifiers.equal_range(intrinsicName)};
+ if (range.first != range.second) {
+ return &range.first->verifier;
+ }
+ return nullptr;
+}
+
+/// Ensure argument verifiers, if any, are run before calling the runtime
+/// wrapper to fold an intrinsic.
+static HostRuntimeWrapper AddArgumentVerifierIfAny(
+ const std::string &intrinsicName, const HostRuntimeFunction &hostFunction) {
+ if (const auto *verifier{findVerifier(intrinsicName)}) {
+ const HostRuntimeFunction *hostFunctionPtr = &hostFunction;
+ return [hostFunctionPtr, verifier](
+ FoldingContext &context, std::vector<Expr<SomeType>> &&args) {
+ const bool validArguments{(*verifier)(args, context)};
+ if (!validArguments) {
+ // Silence fp signal warnings since a more detailed warning about
+ // invalid arguments was already emitted.
+ parser::Messages localBuffer;
+ parser::ContextualMessages localMessages{&localBuffer};
+ FoldingContext localContext{context, localMessages};
+ return hostFunctionPtr->folder(localContext, std::move(args));
+ }
+ return hostFunctionPtr->folder(context, std::move(args));
+ };
+ }
+ return hostFunction.folder;
+}
+
std::optional<HostRuntimeWrapper> GetHostRuntimeWrapper(const std::string &name,
DynamicType resultType, const std::vector<DynamicType> &argTypes) {
if (const auto *hostFunction{SearchHostRuntime(name, resultType, argTypes)}) {
- return hostFunction->folder;
+ return AddArgumentVerifierIfAny(name, *hostFunction);
}
// If no exact match, search with "bigger" types and insert type
// conversions around the folder.
@@ -647,7 +880,8 @@ std::optional<HostRuntimeWrapper> GetHostRuntimeWrapper(const std::string &name,
}
if (const auto *hostFunction{
SearchHostRuntime(name, biggerResultType, biggerArgTypes)}) {
- return [hostFunction, resultType](
+ auto hostFolderWithChecks{AddArgumentVerifierIfAny(name, *hostFunction)};
+ return [hostFunction, resultType, hostFolderWithChecks](
FoldingContext &context, std::vector<Expr<SomeType>> &&args) {
auto nArgs{args.size()};
for (size_t i{0}; i < nArgs; ++i) {
@@ -657,7 +891,7 @@ std::optional<HostRuntimeWrapper> GetHostRuntimeWrapper(const std::string &name,
}
return Fold(context,
ConvertToType(
- resultType, hostFunction->folder(context, std::move(args)))
+ resultType, hostFolderWithChecks(context, std::move(args)))
.value());
};
}
diff --git a/flang/test/Evaluate/folding04.f90 b/flang/test/Evaluate/folding04.f90
index c7815b0340360..cc46e31a33388 100644
--- a/flang/test/Evaluate/folding04.f90
+++ b/flang/test/Evaluate/folding04.f90
@@ -17,24 +17,45 @@ module real_tests
!WARN: warning: division by zero
real(4), parameter :: r4_ninf = -1._4/0._4
- !WARN: warning: invalid argument on evaluation of intrinsic function or operation
+ !WARN: warning: argument is out of range [-1., 1.]
real(4), parameter :: nan_r4_acos1 = acos(1.1)
TEST_ISNAN(nan_r4_acos1)
- !WARN: warning: invalid argument on evaluation of intrinsic function or operation
+ !WARN: warning: argument is out of range [-1., 1.]
real(4), parameter :: nan_r4_acos2 = acos(r4_pmax)
TEST_ISNAN(nan_r4_acos2)
- !WARN: warning: invalid argument on evaluation of intrinsic function or operation
+ !WARN: warning: argument is out of range [-1., 1.]
real(4), parameter :: nan_r4_acos3 = acos(r4_nmax)
TEST_ISNAN(nan_r4_acos3)
- !WARN: warning: invalid argument on evaluation of intrinsic function or operation
+ !WARN: warning: argument is out of range [-1., 1.]
real(4), parameter :: nan_r4_acos4 = acos(r4_ninf)
TEST_ISNAN(nan_r4_acos4)
- !WARN: warning: invalid argument on evaluation of intrinsic function or operation
+ !WARN: warning: argument is out of range [-1., 1.]
real(4), parameter :: nan_r4_acos5 = acos(r4_pinf)
TEST_ISNAN(nan_r4_acos5)
+ !WARN: warning: argument is out of range [-1., 1.]
+ real(8), parameter :: nan_r8_dasin1 = dasin(-1.1_8)
+ TEST_ISNAN(nan_r8_dasin1)
+ !WARN: warning: argument 'x' must be strictly positive
+ real(8), parameter :: nan_r8_dlog1 = dlog(-0.1_8)
+ TEST_ISNAN(nan_r8_dlog1)
+ !WARN: warning: complex argument must be different from zero
+ complex(4), parameter :: c4_clog1 = clog((0., 0.))
!WARN: warning: MOD: P argument is zero
real(4), parameter :: nan_r4_mod = mod(3.5, 0.)
TEST_ISNAN(nan_r4_mod)
+ real(4), parameter :: ok_r4_gamma = gamma(-1.1)
+ !WARN: argument must not be a negative integer or zero
+ real(4), parameter :: r4_gamma1 = gamma(0.)
+ !WARN: argument must not be a negative integer or zero
+ real(4), parameter :: r4_gamma2 = gamma(-1.)
+ real(4), parameter :: ok_r4_log_gamma = log_gamma(-2.001)
+ !WARN: argument must not be a negative integer or zero
+ real(4), parameter :: r4_log_gamma1 = log_gamma(0.)
+ !WARN: argument must not be a negative integer or zero
+ real(4), parameter :: r4_log_gamma2 = log_gamma(-100001.)
+ !WARN: 'x' and 'y' arguments must not be both zero
+ real(4), parameter :: r4_atan2 = atan2(0., 0.)
+
!WARN: warning: overflow on evaluation of intrinsic function or operation
logical, parameter :: test_exp_overflow = exp(256._4).EQ.r4_pinf
contains
>From 59be62c80d89bd92c3f5f1db87cc3348ae2b6c49 Mon Sep 17 00:00:00 2001
From: Peter Steinfeld <psteinfeld at nvidia.com>
Date: Wed, 26 Jun 2024 12:56:13 -0700
Subject: [PATCH 2/2] Just trying to figure out why the Windows build is
failing.
---
flang/test/Evaluate/folding04.f90 | 36 -------------------------------
1 file changed, 36 deletions(-)
diff --git a/flang/test/Evaluate/folding04.f90 b/flang/test/Evaluate/folding04.f90
index cc46e31a33388..1a982b6c5e9a5 100644
--- a/flang/test/Evaluate/folding04.f90
+++ b/flang/test/Evaluate/folding04.f90
@@ -17,44 +17,8 @@ module real_tests
!WARN: warning: division by zero
real(4), parameter :: r4_ninf = -1._4/0._4
- !WARN: warning: argument is out of range [-1., 1.]
- real(4), parameter :: nan_r4_acos1 = acos(1.1)
- TEST_ISNAN(nan_r4_acos1)
- !WARN: warning: argument is out of range [-1., 1.]
- real(4), parameter :: nan_r4_acos2 = acos(r4_pmax)
- TEST_ISNAN(nan_r4_acos2)
- !WARN: warning: argument is out of range [-1., 1.]
- real(4), parameter :: nan_r4_acos3 = acos(r4_nmax)
- TEST_ISNAN(nan_r4_acos3)
- !WARN: warning: argument is out of range [-1., 1.]
- real(4), parameter :: nan_r4_acos4 = acos(r4_ninf)
- TEST_ISNAN(nan_r4_acos4)
- !WARN: warning: argument is out of range [-1., 1.]
- real(4), parameter :: nan_r4_acos5 = acos(r4_pinf)
- TEST_ISNAN(nan_r4_acos5)
- !WARN: warning: argument is out of range [-1., 1.]
- real(8), parameter :: nan_r8_dasin1 = dasin(-1.1_8)
- TEST_ISNAN(nan_r8_dasin1)
- !WARN: warning: argument 'x' must be strictly positive
- real(8), parameter :: nan_r8_dlog1 = dlog(-0.1_8)
- TEST_ISNAN(nan_r8_dlog1)
- !WARN: warning: complex argument must be different from zero
- complex(4), parameter :: c4_clog1 = clog((0., 0.))
- !WARN: warning: MOD: P argument is zero
- real(4), parameter :: nan_r4_mod = mod(3.5, 0.)
- TEST_ISNAN(nan_r4_mod)
real(4), parameter :: ok_r4_gamma = gamma(-1.1)
- !WARN: argument must not be a negative integer or zero
- real(4), parameter :: r4_gamma1 = gamma(0.)
- !WARN: argument must not be a negative integer or zero
- real(4), parameter :: r4_gamma2 = gamma(-1.)
real(4), parameter :: ok_r4_log_gamma = log_gamma(-2.001)
- !WARN: argument must not be a negative integer or zero
- real(4), parameter :: r4_log_gamma1 = log_gamma(0.)
- !WARN: argument must not be a negative integer or zero
- real(4), parameter :: r4_log_gamma2 = log_gamma(-100001.)
- !WARN: 'x' and 'y' arguments must not be both zero
- real(4), parameter :: r4_atan2 = atan2(0., 0.)
!WARN: warning: overflow on evaluation of intrinsic function or operation
logical, parameter :: test_exp_overflow = exp(256._4).EQ.r4_pinf
More information about the flang-commits
mailing list