[flang-commits] [flang] 70969df - [flang] Improve warnings for invalid arguments when folding host runtime (#96807)
via flang-commits
flang-commits at lists.llvm.org
Tue Jul 2 08:38:47 PDT 2024
Author: Pete Steinfeld
Date: 2024-07-02T08:38:43-07:00
New Revision: 70969df73dc9797e1bb219dfb2f6b8ba854bf8ba
URL: https://github.com/llvm/llvm-project/commit/70969df73dc9797e1bb219dfb2f6b8ba854bf8ba
DIFF: https://github.com/llvm/llvm-project/commit/70969df73dc9797e1bb219dfb2f6b8ba854bf8ba.diff
LOG: [flang] Improve warnings for invalid arguments when folding host runtime (#96807)
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.
Note that the original attempt at implementing this contained additional
code in .../llvm-project/flang/test/Evaluate/folding04.f90:
```
!WARN: warning: argument 'x' must be strictly positive
real(8), parameter :: nan_r8_dlog1 = dlog(-0.1_8)
TEST_ISNAN(nan_r8_dlog1)
```
For reasons I don't understand, this additional code caused the Windows
build to fail. So this new version of the update does not contain that
code.
Added:
Modified:
flang/lib/Evaluate/intrinsics-library.cpp
flang/test/Evaluate/folding04.f90
Removed:
################################################################################
diff --git a/flang/lib/Evaluate/intrinsics-library.cpp b/flang/lib/Evaluate/intrinsics-library.cpp
index 7315a7a057b10..c3bd3a501c0c5 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;
+ 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)}) {
+ 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
diff erent 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
diff erent 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"_warn_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"_warn_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..08c5c6eee9b12 100644
--- a/flang/test/Evaluate/folding04.f90
+++ b/flang/test/Evaluate/folding04.f90
@@ -17,24 +17,42 @@ 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: complex argument must be
diff erent 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: warning: argument must not be a negative integer or zero
+ real(4), parameter :: r4_gamma1 = gamma(0.)
+ !WARN: warning: 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: warning: argument must not be a negative integer or zero
+ real(4), parameter :: r4_log_gamma1 = log_gamma(0.)
+ !WARN: warning: argument must not be a negative integer or zero
+ real(4), parameter :: r4_log_gamma2 = log_gamma(-100001.)
+ !WARN: warning: '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
More information about the flang-commits
mailing list