https://github.com/yi-wu-arm updated https://github.com/llvm/llvm-project/pull/74309
>From 14f8c3e38791cc6b06455b8beffe37a6f7105e03 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Mon, 4 Dec 2023 10:32:03 +0000 Subject: [PATCH 01/18] Add SYSTEM runtime and lowering intrinsic support Calls std::system() function and pass the command, cmd on Windows or shell on Linux. Command parameter is required, exitstatus is optional. call system(command) call system(command, exitstatus) --- flang/docs/Intrinsics.md | 2 +- .../flang/Optimizer/Builder/IntrinsicCall.h | 1 + .../flang/Optimizer/Builder/Runtime/Command.h | 5 +++ flang/include/flang/Runtime/command.h | 5 +++ flang/lib/Evaluate/intrinsics.cpp | 16 +++++--- flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 22 ++++++++++ .../lib/Optimizer/Builder/Runtime/Command.cpp | 13 ++++++ flang/runtime/command.cpp | 19 +++++++++ flang/test/Lower/Intrinsics/system.f90 | 39 ++++++++++++++++++ flang/unittests/Runtime/CommandTest.cpp | 41 +++++++++++++++++++ 10 files changed, 157 insertions(+), 6 deletions(-) create mode 100644 flang/test/Lower/Intrinsics/system.f90 diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md index fef2b4ea4dd8c8..871332399628e9 100644 --- a/flang/docs/Intrinsics.md +++ b/flang/docs/Intrinsics.md @@ -751,7 +751,7 @@ This phase currently supports all the intrinsic procedures listed above but the | Object characteristic inquiry functions | ALLOCATED, ASSOCIATED, EXTENDS_TYPE_OF, IS_CONTIGUOUS, PRESENT, RANK, SAME_TYPE, STORAGE_SIZE | | Type inquiry intrinsic functions | BIT_SIZE, DIGITS, EPSILON, HUGE, KIND, MAXEXPONENT, MINEXPONENT, NEW_LINE, PRECISION, RADIX, RANGE, TINY| | Non-standard intrinsic functions | AND, OR, XOR, SHIFT, ZEXT, IZEXT, COSD, SIND, TAND, ACOSD, ASIND, ATAND, ATAN2D, COMPL, EQV, NEQV, INT8, JINT, JNINT, KNINT, QCMPLX, DREAL, DFLOAT, QEXT, QFLOAT, QREAL, DNUM, NUM, JNUM, KNUM, QNUM, RNUM, RAN, RANF, ILEN, SIZEOF, MCLOCK, SECNDS, COTAN, IBCHNG, ISHA, ISHC, ISHL, IXOR, IARG, IARGC, NARGS, GETPID, NUMARG, BADDRESS, IADDR, CACHESIZE, EOF, FP_CLASS, INT_PTR_KIND, ISNAN, MALLOC | -| Intrinsic subroutines |MVBITS (elemental), CPU_TIME, DATE_AND_TIME, EVENT_QUERY, EXECUTE_COMMAND_LINE, GET_COMMAND, GET_COMMAND_ARGUMENT, GET_ENVIRONMENT_VARIABLE, MOVE_ALLOC, RANDOM_INIT, RANDOM_NUMBER, RANDOM_SEED, SYSTEM_CLOCK | +| Intrinsic subroutines |MVBITS (elemental), CPU_TIME, DATE_AND_TIME, EVENT_QUERY, EXECUTE_COMMAND_LINE, GET_COMMAND, GET_COMMAND_ARGUMENT, GET_ENVIRONMENT_VARIABLE, MOVE_ALLOC, RANDOM_INIT, RANDOM_NUMBER, RANDOM_SEED, SYSTEM, SYSTEM_CLOCK | | Atomic intrinsic subroutines | ATOMIC_ADD | | Collective intrinsic subroutines | CO_REDUCE | diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h index 5065f11ae9e726..669d076c3e0e7d 100644 --- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h +++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h @@ -321,6 +321,7 @@ struct IntrinsicLibrary { fir::ExtendedValue genStorageSize(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>); fir::ExtendedValue genSum(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>); + void genSystem(mlir::ArrayRef<fir::ExtendedValue> args); void genSystemClock(llvm::ArrayRef<fir::ExtendedValue>); mlir::Value genTand(mlir::Type, llvm::ArrayRef<mlir::Value>); mlir::Value genTrailz(mlir::Type, llvm::ArrayRef<mlir::Value>); diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Command.h b/flang/include/flang/Optimizer/Builder/Runtime/Command.h index 976fb3aa0b6fbb..9d6a39639844fc 100644 --- a/flang/include/flang/Optimizer/Builder/Runtime/Command.h +++ b/flang/include/flang/Optimizer/Builder/Runtime/Command.h @@ -53,5 +53,10 @@ mlir::Value genGetEnvVariable(fir::FirOpBuilder &, mlir::Location, mlir::Value length, mlir::Value trimName, mlir::Value errmsg); +/// Generate a call to System runtime function which implements +/// the non-standard System GNU extension. +void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command, + mlir::Value exitstat); + } // namespace fir::runtime #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_COMMAND_H diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h index c67d171c8e2f1b..f325faa7bd09fa 100644 --- a/flang/include/flang/Runtime/command.h +++ b/flang/include/flang/Runtime/command.h @@ -55,6 +55,11 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, const Descriptor *value = nullptr, const Descriptor *length = nullptr, bool trim_name = true, const Descriptor *errmsg = nullptr, const char *sourceFile = nullptr, int line = 0); + +// Calls std::system() +void RTNAME(System)(const Descriptor *command = nullptr, + const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr, + int line = 0); } } // namespace Fortran::runtime diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp index c5faf319fafb7d..bb6ac8ac7159a5 100644 --- a/flang/lib/Evaluate/intrinsics.cpp +++ b/flang/lib/Evaluate/intrinsics.cpp @@ -1387,6 +1387,11 @@ static const IntrinsicInterface intrinsicSubroutine[]{ {"get", DefaultInt, Rank::vector, Optionality::optional, common::Intent::Out}}, {}, Rank::elemental, IntrinsicClass::impureSubroutine}, + {"system", + {{"command", DefaultChar, Rank::scalar}, + {"exitstat", AnyInt, Rank::scalar, Optionality::optional, + common::Intent::InOut}}, + {}, Rank::elemental, IntrinsicClass::impureSubroutine}, {"system_clock", {{"count", AnyInt, Rank::scalar, Optionality::optional, common::Intent::Out}, @@ -1885,7 +1890,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match( int elementalRank{0}; for (std::size_t j{0}; j < dummies; ++j) { const IntrinsicDummyArgument &d{dummy[std::min(j, dummyArgPatterns - 1)]}; - if (const ActualArgument *arg{actualForDummy[j]}) { + if (const ActualArgument * arg{actualForDummy[j]}) { bool isAssumedRank{IsAssumedRank(*arg)}; if (isAssumedRank && d.rank != Rank::anyOrAssumedRank && d.rank != Rank::arrayOrAssumedRank) { @@ -2222,7 +2227,8 @@ std::optional<SpecificCall> IntrinsicInterface::Match( case Rank::locReduced: case Rank::scalarIfDim: if (dummy[*dimArg].optionality == Optionality::required) { - if (const Symbol *whole{ + if (const Symbol * + whole{ UnwrapWholeSymbolOrComponentDataRef(actualForDummy[*dimArg])}) { if (IsOptional(*whole) || IsAllocatableOrObjectPointer(whole)) { if (context.languageFeatures().ShouldWarn( @@ -2300,7 +2306,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match( // Rearrange the actual arguments into dummy argument order. ActualArguments rearranged(dummies); for (std::size_t j{0}; j < dummies; ++j) { - if (ActualArgument *arg{actualForDummy[j]}) { + if (ActualArgument * arg{actualForDummy[j]}) { rearranged[j] = std::move(*arg); } } @@ -2937,7 +2943,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) { ok &= CheckForCoindexedObject(context, call.arguments[3], name, "errmsg"); if (call.arguments[0] && call.arguments[1]) { for (int j{0}; j < 2; ++j) { - if (const Symbol *last{GetLastSymbol(call.arguments[j])}; + if (const Symbol * last{GetLastSymbol(call.arguments[j])}; last && !IsAllocatable(last->GetUltimate())) { context.messages().Say(call.arguments[j]->sourceLocation(), "Argument #%d to MOVE_ALLOC must be allocatable"_err_en_US, @@ -2957,7 +2963,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) { const auto &arg{call.arguments[0]}; if (arg) { if (const auto *expr{arg->UnwrapExpr()}) { - if (const Symbol *symbol{UnwrapWholeSymbolDataRef(*expr)}) { + if (const Symbol * symbol{UnwrapWholeSymbolDataRef(*expr)}) { ok = symbol->attrs().test(semantics::Attr::OPTIONAL); } } diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp index 24fdbe97856b3a..8fded510a18012 100644 --- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp +++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp @@ -508,6 +508,10 @@ static constexpr IntrinsicHandler handlers[]{ {"dim", asValue}, {"mask", asBox, handleDynamicOptional}}}, /*isElemental=*/false}, + {"system", + &I::genSystem, + {{{"command", asBox}, {"exitstat", asBox, handleDynamicOptional}}}, + /*isElemental=*/false}, {"system_clock", &I::genSystemClock, {{{"count", asAddr}, {"count_rate", asAddr}, {"count_max", asAddr}}}, @@ -5316,6 +5320,24 @@ IntrinsicLibrary::genSum(mlir::Type resultType, resultType, args); } +// SYSTEM +void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) { + assert(args.size() >= 1); + mlir::Value command = fir::getBase(args[0]); + const fir::ExtendedValue &exitstat = args[1]; + + if (!command) + fir::emitFatalError(loc, "expected COMMAND parameter"); + + mlir::Type boxNoneTy = fir::BoxType::get(builder.getNoneType()); + + mlir::Value exitstatBox = + isStaticallyPresent(exitstat) + ? fir::getBase(exitstat) + : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult(); + + fir::runtime::genSystem(builder, loc, command, exitstatBox); +} // SYSTEM_CLOCK void IntrinsicLibrary::genSystemClock(llvm::ArrayRef<fir::ExtendedValue> args) { assert(args.size() == 3); diff --git a/flang/lib/Optimizer/Builder/Runtime/Command.cpp b/flang/lib/Optimizer/Builder/Runtime/Command.cpp index 1d719e7bbd9a2d..00af35a74bf871 100644 --- a/flang/lib/Optimizer/Builder/Runtime/Command.cpp +++ b/flang/lib/Optimizer/Builder/Runtime/Command.cpp @@ -88,3 +88,16 @@ mlir::Value fir::runtime::genGetEnvVariable(fir::FirOpBuilder &builder, sourceFile, sourceLine); return builder.create<fir::CallOp>(loc, runtimeFunc, args).getResult(0); } + +void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc, + mlir::Value command, mlir::Value exitstat) { + auto runtimeFunc = + fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder); + mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType(); + mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc); + mlir::Value sourceLine = + fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3)); + llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments( + builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine); + builder.create<fir::CallOp>(loc, runtimeFunc, args); +} diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 8e6135b5487c05..a0f71147275fd1 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -282,4 +282,23 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, return StatOk; } +void RTNAME(System)(const Descriptor *command, const Descriptor *exitstat, + const char *sourceFile, int line) { + Terminator terminator{sourceFile, line}; + + if (command) { + RUNTIME_CHECK(terminator, IsValidCharDescriptor(command)); + int status{std::system(command->OffsetElement())}; + if (exitstat) { + RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); +#ifdef _WIN32 + StoreLengthToDescriptor(exitstat, status, terminator); +#else + int exitstatVal{WEXITSTATUS(status)}; + StoreLengthToDescriptor(exitstat, exitstatVal, terminator); +#endif + } + } +} + } // namespace Fortran::runtime diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90 new file mode 100644 index 00000000000000..da9ccd9d22ae2f --- /dev/null +++ b/flang/test/Lower/Intrinsics/system.f90 @@ -0,0 +1,39 @@ +! RUN: bbc -emit-hlfir %s -o - | FileCheck %s + +! CHECK-LABEL: func.func @_QPall_args() { +! CHECK: %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFall_argsEcommand"} +! CHECK-NEXT: %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>) +! CHECK-NEXT: %2 = fir.alloca i32 {bindc_name = "exitval", uniq_name = "_QFall_argsEexitval"} +! CHECK-NEXT: %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) +! CHECK-NEXT: %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> +! CHECK-NEXT: %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32> +! CHECK-NEXT: %6 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>> +! CHECK-NEXT: %c21_i32 = arith.constant 21 : i32 +! CHECK-NEXT: %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> +! CHECK-NEXT: %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none> +! CHECK-NEXT: %9 = fir.convert %6 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> +! CHECK-NEXT: %10 = fir.call @_FortranASystem(%7, %8, %9, %c21_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } +subroutine all_args() +CHARACTER(30) :: command +INTEGER :: exitVal +call system(command, exitVal) +end subroutine all_args + +! CHECK-LABEL: func.func @_QPonly_command() { +! CHECK: %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFonly_commandEcommand"} +! CHECK-NEXT: %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>) +! CHECK-NEXT: %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> +! CHECK-NEXT: %3 = fir.absent !fir.box<none> +! CHECK-NEXT: %4 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>> +! CHECK-NEXT: %c38_i32 = arith.constant 38 : i32 +! CHECK-NEXT: %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> +! CHECK-NEXT: %6 = fir.convert %4 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> +! CHECK-NEXT: %7 = fir.call @_FortranASystem(%5, %3, %6, %c38_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } +subroutine only_command() +CHARACTER(30) :: command +call system(command) +end subroutine only_command diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index 9f66c7924c86e3..f0bbc721a67073 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -46,6 +46,17 @@ static OwningPtr<Descriptor> EmptyIntDescriptor() { return descriptor; } +template <int kind = sizeof(std::int64_t)> +static OwningPtr<Descriptor> IntDescriptor(const int &value) { + OwningPtr<Descriptor> descriptor{Descriptor::Create(TypeCategory::Integer, + kind, nullptr, 0, nullptr, CFI_attribute_allocatable)}; + if (descriptor->Allocate() != 0) { + return nullptr; + } + std::memcpy(descriptor->OffsetElement<int>(), &value, sizeof(int)); + return descriptor; +} + class CommandFixture : public ::testing::Test { protected: CommandFixture(int argc, const char *argv[]) { @@ -227,6 +238,36 @@ TEST_F(ZeroArguments, GetCommandArgument) { TEST_F(ZeroArguments, GetCommand) { CheckCommandValue(commandOnlyArgv, 1); } +TEST_F(ZeroArguments, SystemValidCommandExitStat) { + OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; + OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; + + RTNAME(System)(command.get(), exitStat.get()); + CheckDescriptorEqInt(exitStat.get(), 0); +} + +TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { + OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; + OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; + + RTNAME(System)(command.get(), exitStat.get()); +#ifdef _WIN32 + CheckDescriptorEqInt(exitStat.get(), 1); +#else + CheckDescriptorEqInt(exitStat.get(), 127); +#endif +} + +TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) { + OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; + EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr)); +} + +TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) { + OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; + EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr)); +} + static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"}; class OneArgument : public CommandFixture { protected: >From be27d36ca18910b9cf521da26948a3fd1c03e228 Mon Sep 17 00:00:00 2001 From: Yi Wu <yiw...@wdev-yiwu02.arm.com> Date: Mon, 4 Dec 2023 12:01:23 +0000 Subject: [PATCH 02/18] test fixes --- flang/test/Lower/Intrinsics/system.f90 | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90 index da9ccd9d22ae2f..0c3e5fd63047c6 100644 --- a/flang/test/Lower/Intrinsics/system.f90 +++ b/flang/test/Lower/Intrinsics/system.f90 @@ -7,12 +7,10 @@ ! CHECK-NEXT: %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) ! CHECK-NEXT: %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> ! CHECK-NEXT: %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32> -! CHECK-NEXT: %6 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>> -! CHECK-NEXT: %c21_i32 = arith.constant 21 : i32 +! CHECK: %c19_i32 = arith.constant 19 : i32 ! CHECK-NEXT: %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> ! CHECK-NEXT: %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none> -! CHECK-NEXT: %9 = fir.convert %6 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> -! CHECK-NEXT: %10 = fir.call @_FortranASystem(%7, %8, %9, %c21_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK: %10 = fir.call @_FortranASystem(%7, %8, %9, %c19_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } subroutine all_args() @@ -26,11 +24,9 @@ end subroutine all_args ! CHECK-NEXT: %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>) ! CHECK-NEXT: %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> ! CHECK-NEXT: %3 = fir.absent !fir.box<none> -! CHECK-NEXT: %4 = fir.address_of(@_QQclX6c088457724b07fb671a04391501cd9d) : !fir.ref<!fir.char<1,73>> -! CHECK-NEXT: %c38_i32 = arith.constant 38 : i32 +! CHECK: %c34_i32 = arith.constant 34 : i32 ! CHECK-NEXT: %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> -! CHECK-NEXT: %6 = fir.convert %4 : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> -! CHECK-NEXT: %7 = fir.call @_FortranASystem(%5, %3, %6, %c38_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK: %7 = fir.call @_FortranASystem(%5, %3, %6, %c34_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } subroutine only_command() >From d8215296237586aabc180ed36c4220f58ccef8e3 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Mon, 4 Dec 2023 12:22:38 +0000 Subject: [PATCH 03/18] remove unnecessary clang-format change --- flang/lib/Evaluate/intrinsics.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp index bb6ac8ac7159a5..98dd2b27a08ebf 100644 --- a/flang/lib/Evaluate/intrinsics.cpp +++ b/flang/lib/Evaluate/intrinsics.cpp @@ -1890,7 +1890,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match( int elementalRank{0}; for (std::size_t j{0}; j < dummies; ++j) { const IntrinsicDummyArgument &d{dummy[std::min(j, dummyArgPatterns - 1)]}; - if (const ActualArgument * arg{actualForDummy[j]}) { + if (const ActualArgument *arg{actualForDummy[j]}) { bool isAssumedRank{IsAssumedRank(*arg)}; if (isAssumedRank && d.rank != Rank::anyOrAssumedRank && d.rank != Rank::arrayOrAssumedRank) { @@ -2227,8 +2227,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match( case Rank::locReduced: case Rank::scalarIfDim: if (dummy[*dimArg].optionality == Optionality::required) { - if (const Symbol * - whole{ + if (const Symbol *whole{ UnwrapWholeSymbolOrComponentDataRef(actualForDummy[*dimArg])}) { if (IsOptional(*whole) || IsAllocatableOrObjectPointer(whole)) { if (context.languageFeatures().ShouldWarn( @@ -2306,7 +2305,7 @@ std::optional<SpecificCall> IntrinsicInterface::Match( // Rearrange the actual arguments into dummy argument order. ActualArguments rearranged(dummies); for (std::size_t j{0}; j < dummies; ++j) { - if (ActualArgument * arg{actualForDummy[j]}) { + if (ActualArgument *arg{actualForDummy[j]}) { rearranged[j] = std::move(*arg); } } @@ -2963,7 +2962,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) { const auto &arg{call.arguments[0]}; if (arg) { if (const auto *expr{arg->UnwrapExpr()}) { - if (const Symbol * symbol{UnwrapWholeSymbolDataRef(*expr)}) { + if (const Symbol *symbol{UnwrapWholeSymbolDataRef(*expr)}) { ok = symbol->attrs().test(semantics::Attr::OPTIONAL); } } >From 9a73cb8b5e30a0963dceb3b8f2e6265f400f2f71 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Thu, 7 Dec 2023 12:20:05 +0000 Subject: [PATCH 04/18] required parameter use reference not pointer, ensure command is null-terminated --- flang/include/flang/Runtime/command.h | 2 +- flang/runtime/command.cpp | 37 ++++++++++++++++++------- flang/unittests/Runtime/CommandTest.cpp | 8 +++--- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h index f325faa7bd09fa..e477a2da1c5954 100644 --- a/flang/include/flang/Runtime/command.h +++ b/flang/include/flang/Runtime/command.h @@ -57,7 +57,7 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, const char *sourceFile = nullptr, int line = 0); // Calls std::system() -void RTNAME(System)(const Descriptor *command = nullptr, +void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr, int line = 0); } diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index a0f71147275fd1..709e1cb4904611 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -282,22 +282,39 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, return StatOk; } -void RTNAME(System)(const Descriptor *command, const Descriptor *exitstat, +const char *ensureNullTerminated( + const char *str, size_t length, Terminator &terminator) { + if (length < strlen(str)) { + char *newCmd{(char *)malloc(length + 1)}; + if (newCmd == NULL) { + terminator.Crash("Command not null-terminated, memory allocation failed " + "for null-terminated newCmd."); + } + + strncpy(newCmd, str, length); + newCmd[length] = '\0'; + return newCmd; + } else { + return str; + } +} + +void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, const char *sourceFile, int line) { Terminator terminator{sourceFile, line}; - if (command) { - RUNTIME_CHECK(terminator, IsValidCharDescriptor(command)); - int status{std::system(command->OffsetElement())}; - if (exitstat) { - RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); + const char *newCmd{ensureNullTerminated( + command.OffsetElement(), command.ElementBytes(), terminator)}; + int status{std::system(newCmd)}; + + if (exitstat) { + RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); #ifdef _WIN32 - StoreLengthToDescriptor(exitstat, status, terminator); + StoreLengthToDescriptor(exitstat, status, terminator); #else - int exitstatVal{WEXITSTATUS(status)}; - StoreLengthToDescriptor(exitstat, exitstatVal, terminator); + int exitstatVal{WEXITSTATUS(status)}; + StoreLengthToDescriptor(exitstat, exitstatVal, terminator); #endif - } } } diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index f0bbc721a67073..466e64cf65299e 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -242,7 +242,7 @@ TEST_F(ZeroArguments, SystemValidCommandExitStat) { OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; - RTNAME(System)(command.get(), exitStat.get()); + RTNAME(System)(*command.get(), exitStat.get()); CheckDescriptorEqInt(exitStat.get(), 0); } @@ -250,7 +250,7 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; - RTNAME(System)(command.get(), exitStat.get()); + RTNAME(System)(*command.get(), exitStat.get()); #ifdef _WIN32 CheckDescriptorEqInt(exitStat.get(), 1); #else @@ -260,12 +260,12 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) { OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; - EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr)); + EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr)); } TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) { OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; - EXPECT_NO_FATAL_FAILURE(RTNAME(System)(command.get(), nullptr)); + EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr)); } static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"}; >From da3f993b775c0e9a241a38b73eb06a41ea65ecd3 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Tue, 12 Dec 2023 11:30:16 +0000 Subject: [PATCH 05/18] remove redundant clang-format and minor fixes --- flang/lib/Evaluate/intrinsics.cpp | 2 +- flang/runtime/command.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp index 98dd2b27a08ebf..a1066ef521961e 100644 --- a/flang/lib/Evaluate/intrinsics.cpp +++ b/flang/lib/Evaluate/intrinsics.cpp @@ -2942,7 +2942,7 @@ static bool ApplySpecificChecks(SpecificCall &call, FoldingContext &context) { ok &= CheckForCoindexedObject(context, call.arguments[3], name, "errmsg"); if (call.arguments[0] && call.arguments[1]) { for (int j{0}; j < 2; ++j) { - if (const Symbol * last{GetLastSymbol(call.arguments[j])}; + if (const Symbol *last{GetLastSymbol(call.arguments[j])}; last && !IsAllocatable(last->GetUltimate())) { context.messages().Say(call.arguments[j]->sourceLocation(), "Argument #%d to MOVE_ALLOC must be allocatable"_err_en_US, diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 709e1cb4904611..03efb57e2c8cd7 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -284,7 +284,7 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, const char *ensureNullTerminated( const char *str, size_t length, Terminator &terminator) { - if (length < strlen(str)) { + if (length <= strlen(str)) { char *newCmd{(char *)malloc(length + 1)}; if (newCmd == NULL) { terminator.Crash("Command not null-terminated, memory allocation failed " >From 2dbeb8162750e9a2bed81b09f20cef66c8967587 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Thu, 14 Dec 2023 13:19:55 +0000 Subject: [PATCH 06/18] tests fixes: avoid hard-coded SSA --- .../test/Lower/Intrinsics/system-optional.f90 | 24 +++++++++ flang/test/Lower/Intrinsics/system.f90 | 54 ++++++++++--------- 2 files changed, 52 insertions(+), 26 deletions(-) create mode 100644 flang/test/Lower/Intrinsics/system-optional.f90 diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90 new file mode 100644 index 00000000000000..d9f5a79965a98a --- /dev/null +++ b/flang/test/Lower/Intrinsics/system-optional.f90 @@ -0,0 +1,24 @@ +! RUN: bbc -emit-hlfir %s -o - | FileCheck %s + +! CHECK-LABEL: func.func @_QPall_args( +! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional}, +! CHECK-SAME: %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval", fir.optional}) { +subroutine all_args(command, exitVal) +CHARACTER(*), OPTIONAL :: command +INTEGER, OPTIONAL :: exitVal +call system(command, exitVal) +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) +! CHECK-NEXT: %[[exitvalIsPresent:.*]] = fir.is_present %[[exitvalDeclare]]#0 : (!fir.ref<i32>) -> i1 +! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> +! CHECK-NEXT: %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> +! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<i32> +! CHECK-NEXT: %[[exitvalSelect:.*]] = arith.select %[[exitvalIsPresent]], %[[exitvalBox]], %[[absentBox]] : !fir.box<i32> +! CHECK: %c9_i32 = arith.constant 9 : i32 +! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> +! CHECK-NEXT: %[[exitval:.*]] = fir.convert %[[exitvalSelect]] : (!fir.box<i32>) -> !fir.box<none> +! CHECK: %[[VAL_12:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_11:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } +end subroutine all_args diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90 index 0c3e5fd63047c6..46fc828a544acc 100644 --- a/flang/test/Lower/Intrinsics/system.f90 +++ b/flang/test/Lower/Intrinsics/system.f90 @@ -1,35 +1,37 @@ ! RUN: bbc -emit-hlfir %s -o - | FileCheck %s -! CHECK-LABEL: func.func @_QPall_args() { -! CHECK: %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFall_argsEcommand"} -! CHECK-NEXT: %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>) -! CHECK-NEXT: %2 = fir.alloca i32 {bindc_name = "exitval", uniq_name = "_QFall_argsEexitval"} -! CHECK-NEXT: %3:2 = hlfir.declare %2 {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) -! CHECK-NEXT: %4 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> -! CHECK-NEXT: %5 = fir.embox %3#1 : (!fir.ref<i32>) -> !fir.box<i32> -! CHECK: %c19_i32 = arith.constant 19 : i32 -! CHECK-NEXT: %7 = fir.convert %4 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> -! CHECK-NEXT: %8 = fir.convert %5 : (!fir.box<i32>) -> !fir.box<none> -! CHECK: %10 = fir.call @_FortranASystem(%7, %8, %9, %c19_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none -! CHECK-NEXT: return -! CHECK-NEXT: } -subroutine all_args() -CHARACTER(30) :: command +! CHECK-LABEL: func.func @_QPall_args( +! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}, +! CHECK-SAME: %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval"}) { +subroutine all_args(command, exitVal) +CHARACTER(*) :: command INTEGER :: exitVal call system(command, exitVal) +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) +! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> +! CHECK-NEXT: %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> +! CHECK: %c9_i32 = arith.constant 9 : i32 +! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> +! CHECK-NEXT: %[[exitval:.*]] = fir.convert %[[exitvalBox]] : (!fir.box<i32>) -> !fir.box<none> +! CHECK: %[[VAL_9:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_8:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } end subroutine all_args -! CHECK-LABEL: func.func @_QPonly_command() { -! CHECK: %0 = fir.alloca !fir.char<1,30> {bindc_name = "command", uniq_name = "_QFonly_commandEcommand"} -! CHECK-NEXT: %1:2 = hlfir.declare %0 typeparams %c30 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,30>>, index) -> (!fir.ref<!fir.char<1,30>>, !fir.ref<!fir.char<1,30>>) -! CHECK-NEXT: %2 = fir.embox %1#1 : (!fir.ref<!fir.char<1,30>>) -> !fir.box<!fir.char<1,30>> -! CHECK-NEXT: %3 = fir.absent !fir.box<none> -! CHECK: %c34_i32 = arith.constant 34 : i32 -! CHECK-NEXT: %5 = fir.convert %2 : (!fir.box<!fir.char<1,30>>) -> !fir.box<none> -! CHECK: %7 = fir.call @_FortranASystem(%5, %3, %6, %c34_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-LABEL: func.func @_QPonly_command( +! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}) { +subroutine only_command(command) +CHARACTER(*) :: command +call system(command) +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> +! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<none> +! CHECK: %c27_i32 = arith.constant 27 : i32 +! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> +! CHECK: %[[VAL_7:.*]] = fir.call @_FortranASystem(%[[command]], %[[absentBox]], %[[VAL_6:.*]], %c27_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } -subroutine only_command() -CHARACTER(30) :: command -call system(command) end subroutine only_command >From 27aa276d16fcd754377286318f9738f59695b7e5 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Thu, 14 Dec 2023 13:43:52 +0000 Subject: [PATCH 07/18] move EnsureNullTerminated to tools.h, capitalized and free memory after use --- flang/runtime/command.cpp | 20 ++------------------ flang/runtime/tools.h | 12 ++++++++++++ 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 03efb57e2c8cd7..4db7ed38e8d20a 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -282,28 +282,11 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, return StatOk; } -const char *ensureNullTerminated( - const char *str, size_t length, Terminator &terminator) { - if (length <= strlen(str)) { - char *newCmd{(char *)malloc(length + 1)}; - if (newCmd == NULL) { - terminator.Crash("Command not null-terminated, memory allocation failed " - "for null-terminated newCmd."); - } - - strncpy(newCmd, str, length); - newCmd[length] = '\0'; - return newCmd; - } else { - return str; - } -} - void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, const char *sourceFile, int line) { Terminator terminator{sourceFile, line}; - const char *newCmd{ensureNullTerminated( + const char *newCmd{EnsureNullTerminated( command.OffsetElement(), command.ElementBytes(), terminator)}; int status{std::system(newCmd)}; @@ -315,6 +298,7 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, int exitstatVal{WEXITSTATUS(status)}; StoreLengthToDescriptor(exitstat, exitstatVal, terminator); #endif + FreeMemory((void *)newCmd); } } diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h index ea659190e14391..735acad728422b 100644 --- a/flang/runtime/tools.h +++ b/flang/runtime/tools.h @@ -411,5 +411,17 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from, bool toIsContiguous, bool fromIsContiguous); RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from); +inline RT_API_ATTRS const char *EnsureNullTerminated( + const char *str, size_t length, Terminator &terminator) { + if (length <= std::strlen(str)) { + char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; + std::memcpy(newCmd, str, length); + newCmd[length] = '\0'; + return newCmd; + } else { + return str; + } +} + } // namespace Fortran::runtime #endif // FORTRAN_RUNTIME_TOOLS_H_ >From cb88f6ebe458193cac62199005392fc7bda6e2db Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Thu, 14 Dec 2023 16:59:59 +0000 Subject: [PATCH 08/18] declare in tools.g, define in tools.cpp --- flang/runtime/tools.cpp | 12 ++++++++++++ flang/runtime/tools.h | 13 ++----------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/flang/runtime/tools.cpp b/flang/runtime/tools.cpp index a027559d9f4a74..b46cec9239d428 100644 --- a/flang/runtime/tools.cpp +++ b/flang/runtime/tools.cpp @@ -173,5 +173,17 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) { ShallowCopy(to, from, to.IsContiguous(), from.IsContiguous()); } +RT_API_ATTRS const char *EnsureNullTerminated( + const char *str, size_t length, Terminator &terminator) { + if (length <= std::strlen(str)) { + char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; + std::memcpy(newCmd, str, length); + newCmd[length] = '\0'; + return newCmd; + } else { + return str; + } +} + RT_OFFLOAD_API_GROUP_END } // namespace Fortran::runtime diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h index 735acad728422b..30a9add01ffbce 100644 --- a/flang/runtime/tools.h +++ b/flang/runtime/tools.h @@ -411,17 +411,8 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from, bool toIsContiguous, bool fromIsContiguous); RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from); -inline RT_API_ATTRS const char *EnsureNullTerminated( - const char *str, size_t length, Terminator &terminator) { - if (length <= std::strlen(str)) { - char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; - std::memcpy(newCmd, str, length); - newCmd[length] = '\0'; - return newCmd; - } else { - return str; - } -} +RT_API_ATTRS const char *EnsureNullTerminated( + const char *str, size_t length, Terminator &terminator); } // namespace Fortran::runtime #endif // FORTRAN_RUNTIME_TOOLS_H_ >From fb15f831b25286bb95d09968d181e92893608f18 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Wed, 20 Dec 2023 16:04:42 +0000 Subject: [PATCH 09/18] free memoryin the end --- flang/runtime/command.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 4db7ed38e8d20a..11187b07fc267f 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -298,8 +298,8 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, int exitstatVal{WEXITSTATUS(status)}; StoreLengthToDescriptor(exitstat, exitstatVal, terminator); #endif - FreeMemory((void *)newCmd); } + FreeMemory((void *)newCmd); } } // namespace Fortran::runtime >From d147dbb031083017ab0ee26f1ac2bf00275d6ef1 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Fri, 29 Dec 2023 13:18:54 +0000 Subject: [PATCH 10/18] memory deallocation fixes, and use std::size_t --- flang/runtime/command.cpp | 5 ++++- flang/runtime/tools.cpp | 6 ++++-- flang/runtime/tools.h | 6 +++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 11187b07fc267f..9a21313ecb2e6a 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -299,7 +299,10 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, StoreLengthToDescriptor(exitstat, exitstatVal, terminator); #endif } - FreeMemory((void *)newCmd); + // Deallocate memory if EnsureNullTerminated dynamically allocate a memory + if (newCmd != command.OffsetElement()) { + FreeMemory((void *)newCmd); + } } } // namespace Fortran::runtime diff --git a/flang/runtime/tools.cpp b/flang/runtime/tools.cpp index b46cec9239d428..c9b7a9922d6453 100644 --- a/flang/runtime/tools.cpp +++ b/flang/runtime/tools.cpp @@ -174,8 +174,10 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from) { } RT_API_ATTRS const char *EnsureNullTerminated( - const char *str, size_t length, Terminator &terminator) { - if (length <= std::strlen(str)) { + const char *str, std::size_t length, Terminator &terminator) { + const void *nullTerminatorPos{std::memchr(str, '\0', length)}; + + if (nullTerminatorPos == nullptr) { char *newCmd{(char *)AllocateMemoryOrCrash(terminator, length + 1)}; std::memcpy(newCmd, str, length); newCmd[length] = '\0'; diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h index 30a9add01ffbce..003ac66e7c7c99 100644 --- a/flang/runtime/tools.h +++ b/flang/runtime/tools.h @@ -411,8 +411,12 @@ RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from, bool toIsContiguous, bool fromIsContiguous); RT_API_ATTRS void ShallowCopy(const Descriptor &to, const Descriptor &from); +// Ensures that a character string is null-terminated, allocating a /p length +1 +// size memory for null-terminator if necessary. Returns the original or a newly +// allocated null-terminated string (responsibility for deallocation is on the +// caller). RT_API_ATTRS const char *EnsureNullTerminated( - const char *str, size_t length, Terminator &terminator); + const char *str, std::size_t length, Terminator &terminator); } // namespace Fortran::runtime #endif // FORTRAN_RUNTIME_TOOLS_H_ >From cce16643a918059206f7d8d449aad5364b2e079d Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Tue, 16 Jan 2024 12:08:58 +0000 Subject: [PATCH 11/18] refactor: move SYSTEM from command.cpp to execute.cpp --- .../flang/Optimizer/Builder/Runtime/Command.h | 5 --- .../flang/Optimizer/Builder/Runtime/Execute.h | 5 +++ flang/include/flang/Runtime/command.h | 5 --- flang/include/flang/Runtime/execute.h | 5 +++ .../lib/Optimizer/Builder/Runtime/Command.cpp | 13 ------ .../lib/Optimizer/Builder/Runtime/Execute.cpp | 13 ++++++ flang/runtime/command.cpp | 23 ---------- flang/runtime/execute.cpp | 23 ++++++++++ flang/runtime/tools.h | 44 ------------------- 9 files changed, 46 insertions(+), 90 deletions(-) diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Command.h b/flang/include/flang/Optimizer/Builder/Runtime/Command.h index 9d6a39639844fc..976fb3aa0b6fbb 100644 --- a/flang/include/flang/Optimizer/Builder/Runtime/Command.h +++ b/flang/include/flang/Optimizer/Builder/Runtime/Command.h @@ -53,10 +53,5 @@ mlir::Value genGetEnvVariable(fir::FirOpBuilder &, mlir::Location, mlir::Value length, mlir::Value trimName, mlir::Value errmsg); -/// Generate a call to System runtime function which implements -/// the non-standard System GNU extension. -void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command, - mlir::Value exitstat); - } // namespace fir::runtime #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_COMMAND_H diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h index a1e6ef20876049..d6621f00fafef7 100644 --- a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h +++ b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h @@ -31,5 +31,10 @@ void genExecuteCommandLine(fir::FirOpBuilder &, mlir::Location, mlir::Value exitstat, mlir::Value cmdstat, mlir::Value cmdmsg); +/// Generate a call to System runtime function which implements +/// the non-standard System GNU extension. +void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command, + mlir::Value exitstat); + } // namespace fir::runtime #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_EXECUTE_H diff --git a/flang/include/flang/Runtime/command.h b/flang/include/flang/Runtime/command.h index e477a2da1c5954..c67d171c8e2f1b 100644 --- a/flang/include/flang/Runtime/command.h +++ b/flang/include/flang/Runtime/command.h @@ -55,11 +55,6 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, const Descriptor *value = nullptr, const Descriptor *length = nullptr, bool trim_name = true, const Descriptor *errmsg = nullptr, const char *sourceFile = nullptr, int line = 0); - -// Calls std::system() -void RTNAME(System)(const Descriptor &command, - const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr, - int line = 0); } } // namespace Fortran::runtime diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h index ca137b9d1823c4..9d40910dccd679 100644 --- a/flang/include/flang/Runtime/execute.h +++ b/flang/include/flang/Runtime/execute.h @@ -23,6 +23,11 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true, const Descriptor *exitstat = nullptr, const Descriptor *cmdstat = nullptr, const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr, int line = 0); + +// Calls std::system() +void RTNAME(System)(const Descriptor &command, + const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr, + int line = 0); } } // namespace Fortran::runtime diff --git a/flang/lib/Optimizer/Builder/Runtime/Command.cpp b/flang/lib/Optimizer/Builder/Runtime/Command.cpp index 00af35a74bf871..1d719e7bbd9a2d 100644 --- a/flang/lib/Optimizer/Builder/Runtime/Command.cpp +++ b/flang/lib/Optimizer/Builder/Runtime/Command.cpp @@ -88,16 +88,3 @@ mlir::Value fir::runtime::genGetEnvVariable(fir::FirOpBuilder &builder, sourceFile, sourceLine); return builder.create<fir::CallOp>(loc, runtimeFunc, args).getResult(0); } - -void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc, - mlir::Value command, mlir::Value exitstat) { - auto runtimeFunc = - fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder); - mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType(); - mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc); - mlir::Value sourceLine = - fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3)); - llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments( - builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine); - builder.create<fir::CallOp>(loc, runtimeFunc, args); -} diff --git a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp index 71ee3996ac0da7..ce16b5de65e293 100644 --- a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp +++ b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp @@ -42,3 +42,16 @@ void fir::runtime::genExecuteCommandLine(fir::FirOpBuilder &builder, sourceFile, sourceLine); builder.create<fir::CallOp>(loc, runtimeFunc, args); } + +void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc, + mlir::Value command, mlir::Value exitstat) { + auto runtimeFunc = + fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder); + mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType(); + mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc); + mlir::Value sourceLine = + fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3)); + llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments( + builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine); + builder.create<fir::CallOp>(loc, runtimeFunc, args); +} diff --git a/flang/runtime/command.cpp b/flang/runtime/command.cpp index 5b2837c96f694a..7c44890545bd3f 100644 --- a/flang/runtime/command.cpp +++ b/flang/runtime/command.cpp @@ -241,27 +241,4 @@ std::int32_t RTNAME(GetEnvVariable)(const Descriptor &name, return StatOk; } -void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, - const char *sourceFile, int line) { - Terminator terminator{sourceFile, line}; - - const char *newCmd{EnsureNullTerminated( - command.OffsetElement(), command.ElementBytes(), terminator)}; - int status{std::system(newCmd)}; - - if (exitstat) { - RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); -#ifdef _WIN32 - StoreLengthToDescriptor(exitstat, status, terminator); -#else - int exitstatVal{WEXITSTATUS(status)}; - StoreLengthToDescriptor(exitstat, exitstatVal, terminator); -#endif - } - // Deallocate memory if EnsureNullTerminated dynamically allocate a memory - if (newCmd != command.OffsetElement()) { - FreeMemory((void *)newCmd); - } -} - } // namespace Fortran::runtime diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index d38cb8384bc864..a2791a8e155207 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -204,4 +204,27 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, } } +void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, + const char *sourceFile, int line) { + Terminator terminator{sourceFile, line}; + + char *newCmd{EnsureNullTerminated( + command.OffsetElement(), command.ElementBytes(), terminator)}; + int status{std::system(newCmd)}; + + if (exitstat) { + RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); +#ifdef _WIN32 + StoreLengthToDescriptor(exitstat, status, terminator); +#else + int exitstatVal{WEXITSTATUS(status)}; + StoreIntToDescriptor(exitstat, exitstatVal, terminator); +#endif + } + // Deallocate memory if EnsureNullTerminated dynamically allocate a memory + if (newCmd != command.OffsetElement()) { + FreeMemory(static_cast<char *>(newCmd)); + } +} + } // namespace Fortran::runtime diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h index 514a0a926f7541..86c06149dc4e4a 100644 --- a/flang/runtime/tools.h +++ b/flang/runtime/tools.h @@ -481,50 +481,6 @@ RT_API_ATTRS void CopyAndPad( } } -// Ensures that a character string is null-terminated, allocating a /p length +1 -// size memory for null-terminator if necessary. Returns the original or a newly -// allocated null-terminated string (responsibility for deallocation is on the -// caller). -RT_API_ATTRS const char *EnsureNullTerminated( - const char *str, std::size_t length, Terminator &terminator); - -RT_API_ATTRS bool IsValidCharDescriptor(const Descriptor *value); - -RT_API_ATTRS bool IsValidIntDescriptor(const Descriptor *intVal); - -// Copy a null-terminated character array \p rawValue to descriptor \p value. -// The copy starts at the given \p offset, if not present then start at 0. -// If descriptor `errmsg` is provided, error messages will be stored to it. -// Returns stats specified in standard. -RT_API_ATTRS std::int32_t CopyCharsToDescriptor(const Descriptor &value, - const char *rawValue, std::size_t rawValueLength, - const Descriptor *errmsg = nullptr, std::size_t offset = 0); - -RT_API_ATTRS void StoreIntToDescriptor( - const Descriptor *length, std::int64_t value, Terminator &terminator); - -// Defines a utility function for copying and padding characters -template <typename TO, typename FROM> -RT_API_ATTRS void CopyAndPad( - TO *to, const FROM *from, std::size_t toChars, std::size_t fromChars) { - if constexpr (sizeof(TO) != sizeof(FROM)) { - std::size_t copyChars{std::min(toChars, fromChars)}; - for (std::size_t j{0}; j < copyChars; ++j) { - to[j] = from[j]; - } - for (std::size_t j{copyChars}; j < toChars; ++j) { - to[j] = static_cast<TO>(' '); - } - } else if (toChars <= fromChars) { - std::memcpy(to, from, toChars * sizeof(TO)); - } else { - std::memcpy(to, from, std::min(toChars, fromChars) * sizeof(TO)); - for (std::size_t j{fromChars}; j < toChars; ++j) { - to[j] = static_cast<TO>(' '); - } - } -} - } // namespace Fortran::runtime #endif // FORTRAN_RUNTIME_TOOLS_H_ >From 31c3a42490548d702edd5fffb34eaebf9ad0737c Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Tue, 16 Jan 2024 12:14:33 +0000 Subject: [PATCH 12/18] clang format and change exitstat type from AnyInt to DefaultInt to match gfortran gnu standard --- flang/lib/Evaluate/intrinsics.cpp | 2 +- flang/runtime/tools.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp index 639db1808f126d..068b3cfafcbc11 100644 --- a/flang/lib/Evaluate/intrinsics.cpp +++ b/flang/lib/Evaluate/intrinsics.cpp @@ -1389,7 +1389,7 @@ static const IntrinsicInterface intrinsicSubroutine[]{ {}, Rank::elemental, IntrinsicClass::impureSubroutine}, {"system", {{"command", DefaultChar, Rank::scalar}, - {"exitstat", AnyInt, Rank::scalar, Optionality::optional, + {"exitstat", DefaultInt, Rank::scalar, Optionality::optional, common::Intent::InOut}}, {}, Rank::elemental, IntrinsicClass::impureSubroutine}, {"system_clock", diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h index 86c06149dc4e4a..89e5069995748b 100644 --- a/flang/runtime/tools.h +++ b/flang/runtime/tools.h @@ -481,6 +481,5 @@ RT_API_ATTRS void CopyAndPad( } } - } // namespace Fortran::runtime #endif // FORTRAN_RUNTIME_TOOLS_H_ >From 25c26f97d28e5a793e8469c5ce2a3534164eb93d Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Tue, 16 Jan 2024 14:21:31 +0000 Subject: [PATCH 13/18] Windows fixes --- flang/runtime/execute.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index a2791a8e155207..1aef04ebfc2a02 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -215,13 +215,13 @@ void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, if (exitstat) { RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); #ifdef _WIN32 - StoreLengthToDescriptor(exitstat, status, terminator); + StoreIntToDescriptor(exitstat, status, terminator); #else int exitstatVal{WEXITSTATUS(status)}; StoreIntToDescriptor(exitstat, exitstatVal, terminator); #endif } - // Deallocate memory if EnsureNullTerminated dynamically allocate a memory + // Deallocate memory if EnsureNullTerminated dynamically allocated memory if (newCmd != command.OffsetElement()) { FreeMemory(static_cast<char *>(newCmd)); } >From 836cd28213b11e7d90a183a3553e2bccf8255ad1 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Tue, 16 Jan 2024 15:06:52 +0000 Subject: [PATCH 14/18] reuse ExecuteCommandLine runtime library and lowering --- .../flang/Optimizer/Builder/Runtime/Execute.h | 5 --- flang/include/flang/Runtime/execute.h | 4 --- flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 14 ++++++-- .../lib/Optimizer/Builder/Runtime/Execute.cpp | 13 -------- flang/runtime/execute.cpp | 3 ++ flang/unittests/Runtime/CommandTest.cpp | 32 ++++++++++++++++--- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h index d6621f00fafef7..a1e6ef20876049 100644 --- a/flang/include/flang/Optimizer/Builder/Runtime/Execute.h +++ b/flang/include/flang/Optimizer/Builder/Runtime/Execute.h @@ -31,10 +31,5 @@ void genExecuteCommandLine(fir::FirOpBuilder &, mlir::Location, mlir::Value exitstat, mlir::Value cmdstat, mlir::Value cmdmsg); -/// Generate a call to System runtime function which implements -/// the non-standard System GNU extension. -void genSystem(fir::FirOpBuilder &, mlir::Location, mlir::Value command, - mlir::Value exitstat); - } // namespace fir::runtime #endif // FORTRAN_OPTIMIZER_BUILDER_RUNTIME_EXECUTE_H diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h index 9d40910dccd679..ae8655f2484e49 100644 --- a/flang/include/flang/Runtime/execute.h +++ b/flang/include/flang/Runtime/execute.h @@ -24,10 +24,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true, const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr, int line = 0); -// Calls std::system() -void RTNAME(System)(const Descriptor &command, - const Descriptor *exitstat = nullptr, const char *sourceFile = nullptr, - int line = 0); } } // namespace Fortran::runtime diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp index f57f241badde3a..1c1889396820b6 100644 --- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp +++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp @@ -5923,7 +5923,7 @@ IntrinsicLibrary::genSum(mlir::Type resultType, // SYSTEM void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) { - assert(args.size() >= 1); + assert(args.size() >= 1 && args.size()<=2); mlir::Value command = fir::getBase(args[0]); const fir::ExtendedValue &exitstat = args[1]; @@ -5932,13 +5932,23 @@ void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) { mlir::Type boxNoneTy = fir::BoxType::get(builder.getNoneType()); + mlir::Value waitBool = builder.createBool(loc, true); mlir::Value exitstatBox = isStaticallyPresent(exitstat) ? fir::getBase(exitstat) : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult(); - fir::runtime::genSystem(builder, loc, command, exitstatBox); + // Create a dummmy cmdstat to prevent EXECUTE_COMMAND_LINE terminate itself + // when cmdstat is assigned with a non-zero value but not present + mlir::Value cmdstatBox = builder.createBox(loc, builder.createIntegerConstant(loc, builder.getIndexType(), 0)); + + mlir::Value cmdmsgBox = + builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult(); + + fir::runtime::genExecuteCommandLine(builder, loc, command, waitBool, + exitstatBox, cmdstatBox, cmdmsgBox); } + // SYSTEM_CLOCK void IntrinsicLibrary::genSystemClock(llvm::ArrayRef<fir::ExtendedValue> args) { assert(args.size() == 3); diff --git a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp index ce16b5de65e293..71ee3996ac0da7 100644 --- a/flang/lib/Optimizer/Builder/Runtime/Execute.cpp +++ b/flang/lib/Optimizer/Builder/Runtime/Execute.cpp @@ -42,16 +42,3 @@ void fir::runtime::genExecuteCommandLine(fir::FirOpBuilder &builder, sourceFile, sourceLine); builder.create<fir::CallOp>(loc, runtimeFunc, args); } - -void fir::runtime::genSystem(fir::FirOpBuilder &builder, mlir::Location loc, - mlir::Value command, mlir::Value exitstat) { - auto runtimeFunc = - fir::runtime::getRuntimeFunc<mkRTKey(System)>(loc, builder); - mlir::FunctionType runtimeFuncTy = runtimeFunc.getFunctionType(); - mlir::Value sourceFile = fir::factory::locationToFilename(builder, loc); - mlir::Value sourceLine = - fir::factory::locationToLineNo(builder, loc, runtimeFuncTy.getInput(3)); - llvm::SmallVector<mlir::Value> args = fir::runtime::createArguments( - builder, loc, runtimeFuncTy, command, exitstat, sourceFile, sourceLine); - builder.create<fir::CallOp>(loc, runtimeFunc, args); -} diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index 1aef04ebfc2a02..1ad09e2864a89f 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -25,6 +25,8 @@ #include <unistd.h> #endif +#include <stdio.h> + namespace Fortran::runtime { // cmdstat specified in 16.9.73 @@ -126,6 +128,7 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, if (cmdstat) { RUNTIME_CHECK(terminator, IsValidIntDescriptor(cmdstat)); + printf("cmdstat found. \n"); // Assigned 0 as specifed in standard, if error then overwrite StoreIntToDescriptor(cmdstat, CMD_EXECUTED, terminator); } diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index a99501ab5b245c..ce285629580884 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -405,18 +405,30 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) { } TEST_F(ZeroArguments, SystemValidCommandExitStat) { + // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime + OwningPtr<Descriptor> cmdStat{IntDescriptor(202)}; + bool wait{true}; + // setup finished + OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; - RTNAME(System)(*command.get(), exitStat.get()); + RTNAME(ExecuteCommandLine) + (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr); CheckDescriptorEqInt(exitStat.get(), 0); } TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { + // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime + OwningPtr<Descriptor> cmdStat{IntDescriptor(202)}; + bool wait{true}; + // setup finished + OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; OwningPtr<Descriptor> exitStat{EmptyIntDescriptor()}; - RTNAME(System)(*command.get(), exitStat.get()); + RTNAME(ExecuteCommandLine) + (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr); #ifdef _WIN32 CheckDescriptorEqInt(exitStat.get(), 1); #else @@ -425,13 +437,25 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { } TEST_F(ZeroArguments, SystemValidCommandOptionalExitStat) { + // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime + OwningPtr<Descriptor> cmdStat{IntDescriptor(202)}; + bool wait{true}; + // setup finished + OwningPtr<Descriptor> command{CharDescriptor("echo hi")}; - EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr)); + EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)( + *command.get(), wait, nullptr, cmdStat.get(), nullptr)); } TEST_F(ZeroArguments, SystemInvalidCommandOptionalExitStat) { + // envrionment setup for SYSTEM from EXECUTE_COMMAND_LINE runtime + OwningPtr<Descriptor> cmdStat{IntDescriptor(202)}; + bool wait{true}; + // setup finished + OwningPtr<Descriptor> command{CharDescriptor("InvalidCommand")}; - EXPECT_NO_FATAL_FAILURE(RTNAME(System)(*command.get(), nullptr)); + EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)( + *command.get(), wait, nullptr, cmdStat.get(), nullptr);); } static const char *oneArgArgv[]{"aProgram", "anArgumentOfLength20"}; >From 1b908adafed5f75b5fd6bcf52d3cd473df070bf6 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Wed, 17 Jan 2024 15:26:55 +0000 Subject: [PATCH 15/18] add tests --- flang/lib/Optimizer/Builder/IntrinsicCall.cpp | 12 +++- .../test/Lower/Intrinsics/system-optional.f90 | 42 +++++++----- flang/test/Lower/Intrinsics/system.f90 | 64 ++++++++++++------- 3 files changed, 75 insertions(+), 43 deletions(-) diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp index 1c1889396820b6..b95e6485c13fbc 100644 --- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp +++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp @@ -5923,7 +5923,7 @@ IntrinsicLibrary::genSum(mlir::Type resultType, // SYSTEM void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) { - assert(args.size() >= 1 && args.size()<=2); + assert(args.size() >= 1 && args.size() <= 2); mlir::Value command = fir::getBase(args[0]); const fir::ExtendedValue &exitstat = args[1]; @@ -5939,8 +5939,14 @@ void IntrinsicLibrary::genSystem(llvm::ArrayRef<fir::ExtendedValue> args) { : builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult(); // Create a dummmy cmdstat to prevent EXECUTE_COMMAND_LINE terminate itself - // when cmdstat is assigned with a non-zero value but not present - mlir::Value cmdstatBox = builder.createBox(loc, builder.createIntegerConstant(loc, builder.getIndexType(), 0)); + // when cmdstat is assigned with a non-zero value but not present + mlir::Value tempValue = + builder.createIntegerConstant(loc, builder.getI2Type(), 0); + mlir::Value temp = builder.createTemporary(loc, builder.getI2Type()); + mlir::Value castVal = + builder.createConvert(loc, builder.getI2Type(), tempValue); + builder.create<fir::StoreOp>(loc, castVal, temp); + mlir::Value cmdstatBox = builder.createBox(loc, temp); mlir::Value cmdmsgBox = builder.create<fir::AbsentOp>(loc, boxNoneTy).getResult(); diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90 index d9f5a79965a98a..2fbdfe4b9f5775 100644 --- a/flang/test/Lower/Intrinsics/system-optional.f90 +++ b/flang/test/Lower/Intrinsics/system-optional.f90 @@ -1,24 +1,34 @@ ! RUN: bbc -emit-hlfir %s -o - | FileCheck %s ! CHECK-LABEL: func.func @_QPall_args( -! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional}, -! CHECK-SAME: %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval", fir.optional}) { -subroutine all_args(command, exitVal) +! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command", fir.optional}, +! CHECK-SAME: %[[exitstatArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitstat", fir.optional}) { +subroutine all_args(command, exitstat) CHARACTER(*), OPTIONAL :: command -INTEGER, OPTIONAL :: exitVal -call system(command, exitVal) -! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) -! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) -! CHECK-NEXT: %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) -! CHECK-NEXT: %[[exitvalIsPresent:.*]] = fir.is_present %[[exitvalDeclare]]#0 : (!fir.ref<i32>) -> i1 +INTEGER, OPTIONAL :: exitstat +call system(command, exitstat) + +! CHECK-NEXT: %[[cmdstatVal:.*]] = fir.alloca i2 +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[exitstatDeclare:.*]]:2 = hlfir.declare %[[exitstatArg]] {fortran_attrs = #fir.var_attrs<optional>, uniq_name = "_QFall_argsEexitstat"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) +! CHECK-NEXT: %[[exitstatIsPresent:.*]] = fir.is_present %[[exitstatDeclare]]#0 : (!fir.ref<i32>) -> i1 ! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> -! CHECK-NEXT: %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> -! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<i32> -! CHECK-NEXT: %[[exitvalSelect:.*]] = arith.select %[[exitvalIsPresent]], %[[exitvalBox]], %[[absentBox]] : !fir.box<i32> -! CHECK: %c9_i32 = arith.constant 9 : i32 +! CHECK-NEXT: %[[exitstatBox:.*]] = fir.embox %[[exitstatDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> +! CHECK-NEXT: %[[absentIntBox:.*]] = fir.absent !fir.box<i32> +! CHECK-NEXT: %[[exitstatRealBox:.*]] = arith.select %[[exitstatIsPresent]], %[[exitstatBox]], %[[absentIntBox]] : !fir.box<i32> +! CHECK-NEXT: %[[true:.*]] = arith.constant true +! CHECK-NEXT: %[[c0_i2:.*]] = arith.constant 0 : i2 +! CHECK-NEXT: fir.store %[[c0_i2]] to %[[cmdstatVal]] : !fir.ref<i2> +! CHECK-NEXT: %[[cmdstatBox:.*]] = fir.embox %[[cmdstatVal]] : (!fir.ref<i2>) -> !fir.box<i2> +! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<none> +! CHECK: %[[c9_i32:.*]] = arith.constant 9 : i32 ! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> -! CHECK-NEXT: %[[exitval:.*]] = fir.convert %[[exitvalSelect]] : (!fir.box<i32>) -> !fir.box<none> -! CHECK: %[[VAL_12:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_11:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: %[[exitstat:.*]] = fir.convert %[[exitstatRealBox]] : (!fir.box<i32>) -> !fir.box<none> +! CHECK-NEXT: %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> +! CHECK-NEXT: %[[VAL_15:.*]] = fir.convert %[[VAL_11:.*]] : (!fir.ref<!fir.char<1,82>>) -> !fir.ref<i8> +! CHECK-NEXT: %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return -! CHECK-NEXT: } +! CHECK-NEXT: } + end subroutine all_args diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90 index 46fc828a544acc..37034bbc0c4f84 100644 --- a/flang/test/Lower/Intrinsics/system.f90 +++ b/flang/test/Lower/Intrinsics/system.f90 @@ -2,22 +2,30 @@ ! CHECK-LABEL: func.func @_QPall_args( ! CHECK-SAME: %[[commandArg:.*]]: !fir.boxchar<1> {fir.bindc_name = "command"}, -! CHECK-SAME: %[[exitvalArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitval"}) { -subroutine all_args(command, exitVal) +! CHECK-SAME: %[[exitstatArg:.*]]: !fir.ref<i32> {fir.bindc_name = "exitstat"}) { +subroutine all_args(command, exitstat) CHARACTER(*) :: command -INTEGER :: exitVal -call system(command, exitVal) -! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) -! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) -! CHECK-NEXT: %[[exitvalDeclare:.*]]:2 = hlfir.declare %[[exitvalArg]] {uniq_name = "_QFall_argsEexitval"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) -! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> -! CHECK-NEXT: %[[exitvalBox:.*]] = fir.embox %[[exitvalDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> -! CHECK: %c9_i32 = arith.constant 9 : i32 -! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> -! CHECK-NEXT: %[[exitval:.*]] = fir.convert %[[exitvalBox]] : (!fir.box<i32>) -> !fir.box<none> -! CHECK: %[[VAL_9:.*]] = fir.call @_FortranASystem(%[[command]], %[[exitval]], %[[VAL_8:.*]], %c9_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none -! CHECK-NEXT: return -! CHECK-NEXT: } +INTEGER :: exitstat +call system(command, exitstat) +! CHECK-NEXT: %[[cmdststVal:.*]] = fir.alloca i2 +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFall_argsEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[exitstatDeclare:.*]]:2 = hlfir.declare %[[exitstatArg]] {uniq_name = "_QFall_argsEexitstat"} : (!fir.ref<i32>) -> (!fir.ref<i32>, !fir.ref<i32>) +! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> +! CHECK-NEXT: %[[exitstatBox:.*]] = fir.embox %[[exitstatDeclare]]#1 : (!fir.ref<i32>) -> !fir.box<i32> +! CHECK-NEXT: %[[true:.*]] = arith.constant true +! CHECK-NEXT: %[[c0_i2:.*]] = arith.constant 0 : i2 +! CHECK-NEXT: fir.store %[[c0_i2:.*]] to %[[cmdststVal]] : !fir.ref<i2> +! CHECK-NEXT: %[[cmdstatBox:.*]] = fir.embox %[[cmdststVal]] : (!fir.ref<i2>) -> !fir.box<i2> +! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<none> +! CHECK: %[[c9_i32:.*]] = arith.constant 9 : i32 +! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> +! CHECK-NEXT: %[[exitstat:.*]] = fir.convert %[[exitstatBox]] : (!fir.box<i32>) -> !fir.box<none> +! CHECK-NEXT: %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> +! CHECK-NEXT: %[[VAL_12:.*]] = fir.convert %[[VAL_8:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> +! CHECK-NEXT: %[[VAL_13:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_12]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } end subroutine all_args ! CHECK-LABEL: func.func @_QPonly_command( @@ -25,13 +33,21 @@ end subroutine all_args subroutine only_command(command) CHARACTER(*) :: command call system(command) -! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %[[commandArg]] : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) -! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) -! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> -! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<none> -! CHECK: %c27_i32 = arith.constant 27 : i32 -! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> -! CHECK: %[[VAL_7:.*]] = fir.call @_FortranASystem(%[[command]], %[[absentBox]], %[[VAL_6:.*]], %c27_i32) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none -! CHECK-NEXT: return -! CHECK-NEXT: } +! CHECK-NEXT: %[[cmdstatVal:.*]] = fir.alloca i2 +! CHECK-NEXT: %[[commandUnbox:.*]]:2 = fir.unboxchar %arg0 : (!fir.boxchar<1>) -> (!fir.ref<!fir.char<1,?>>, index) +! CHECK-NEXT: %[[commandDeclare:.*]]:2 = hlfir.declare %[[commandUnbox]]#0 typeparams %[[commandUnbox]]#1 {uniq_name = "_QFonly_commandEcommand"} : (!fir.ref<!fir.char<1,?>>, index) -> (!fir.boxchar<1>, !fir.ref<!fir.char<1,?>>) +! CHECK-NEXT: %[[commandBox:.*]] = fir.embox %[[commandDeclare]]#1 typeparams %[[commandUnbox]]#1 : (!fir.ref<!fir.char<1,?>>, index) -> !fir.box<!fir.char<1,?>> +! CHECK-NEXT: %[[true:.*]] = arith.constant true +! CHECK-NEXT: %[[absentBox:.*]] = fir.absent !fir.box<none> +! CHECK-NEXT: %[[c0_i2:.*]] = arith.constant 0 : i2 +! CHECK-NEXT: fir.store %[[c0_i2:.*]] to %[[cmdstatVal]] : !fir.ref<i2> +! CHECK-NEXT: %[[cmdstatBox:.*]] = fir.embox %[[cmdstatVal]] : (!fir.ref<i2>) -> !fir.box<i2> +! CHECK-NEXT: %[[absentBox2:.*]] = fir.absent !fir.box<none> +! CHECK: %[[c35_i32:.*]] = arith.constant 35 : i32 +! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> +! CHECK-NEXT: %[[cmdstst:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> +! CHECK-NEXT: %[[VAL_10:.*]]0 = fir.convert %[[VAL_7:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> +! CHECK-NEXT: %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10]]0, %[[c35_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK-NEXT: return +! CHECK-NEXT: } end subroutine only_command >From da6089d8a10965f08984da41e1d713b7d08cb853 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Wed, 17 Jan 2024 15:34:18 +0000 Subject: [PATCH 16/18] remove debug leftover and test fixes based on #78302 --- flang/include/flang/Runtime/execute.h | 1 - flang/runtime/execute.cpp | 3 --- flang/unittests/Runtime/CommandTest.cpp | 12 ++++++------ 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/flang/include/flang/Runtime/execute.h b/flang/include/flang/Runtime/execute.h index ae8655f2484e49..ca137b9d1823c4 100644 --- a/flang/include/flang/Runtime/execute.h +++ b/flang/include/flang/Runtime/execute.h @@ -23,7 +23,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait = true, const Descriptor *exitstat = nullptr, const Descriptor *cmdstat = nullptr, const Descriptor *cmdmsg = nullptr, const char *sourceFile = nullptr, int line = 0); - } } // namespace Fortran::runtime diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index 1ad09e2864a89f..1aef04ebfc2a02 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -25,8 +25,6 @@ #include <unistd.h> #endif -#include <stdio.h> - namespace Fortran::runtime { // cmdstat specified in 16.9.73 @@ -128,7 +126,6 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, if (cmdstat) { RUNTIME_CHECK(terminator, IsValidIntDescriptor(cmdstat)); - printf("cmdstat found. \n"); // Assigned 0 as specifed in standard, if error then overwrite StoreIntToDescriptor(cmdstat, CMD_EXECUTED, terminator); } diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index ce285629580884..0b808dbef1ad13 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -386,8 +386,8 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) { RTNAME(ExecuteCommandLine) (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get()); - CheckDescriptorEqInt(exitStat.get(), 404); - CheckDescriptorEqInt(cmdStat.get(), 0); + CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404); + CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0); CheckDescriptorEqStr(cmdMsg.get(), "No change"); } @@ -400,7 +400,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) { EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)( *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get())); - CheckDescriptorEqInt(exitStat.get(), 404); + CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404); CheckDescriptorEqStr(cmdMsg.get(), "No change"); } @@ -415,7 +415,7 @@ TEST_F(ZeroArguments, SystemValidCommandExitStat) { RTNAME(ExecuteCommandLine) (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr); - CheckDescriptorEqInt(exitStat.get(), 0); + CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 0); } TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { @@ -430,9 +430,9 @@ TEST_F(ZeroArguments, SystemInvalidCommandExitStat) { RTNAME(ExecuteCommandLine) (*command.get(), wait, exitStat.get(), cmdStat.get(), nullptr); #ifdef _WIN32 - CheckDescriptorEqInt(exitStat.get(), 1); + CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1); #else - CheckDescriptorEqInt(exitStat.get(), 127); + CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 127); #endif } >From ceac5e97ecabbf78df3b2cd3dc9acec6a2d7d122 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Wed, 17 Jan 2024 15:36:59 +0000 Subject: [PATCH 17/18] more code clean up --- flang/runtime/execute.cpp | 23 ----------------------- flang/unittests/Runtime/CommandTest.cpp | 6 +++--- 2 files changed, 3 insertions(+), 26 deletions(-) diff --git a/flang/runtime/execute.cpp b/flang/runtime/execute.cpp index 1aef04ebfc2a02..d38cb8384bc864 100644 --- a/flang/runtime/execute.cpp +++ b/flang/runtime/execute.cpp @@ -204,27 +204,4 @@ void RTNAME(ExecuteCommandLine)(const Descriptor &command, bool wait, } } -void RTNAME(System)(const Descriptor &command, const Descriptor *exitstat, - const char *sourceFile, int line) { - Terminator terminator{sourceFile, line}; - - char *newCmd{EnsureNullTerminated( - command.OffsetElement(), command.ElementBytes(), terminator)}; - int status{std::system(newCmd)}; - - if (exitstat) { - RUNTIME_CHECK(terminator, IsValidIntDescriptor(exitstat)); -#ifdef _WIN32 - StoreIntToDescriptor(exitstat, status, terminator); -#else - int exitstatVal{WEXITSTATUS(status)}; - StoreIntToDescriptor(exitstat, exitstatVal, terminator); -#endif - } - // Deallocate memory if EnsureNullTerminated dynamically allocated memory - if (newCmd != command.OffsetElement()) { - FreeMemory(static_cast<char *>(newCmd)); - } -} - } // namespace Fortran::runtime diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp index 0b808dbef1ad13..afc2b76bc3d5a3 100644 --- a/flang/unittests/Runtime/CommandTest.cpp +++ b/flang/unittests/Runtime/CommandTest.cpp @@ -386,8 +386,8 @@ TEST_F(ZeroArguments, ECLValidCommandAndExitStatNoChangeAndCMDStatusSetAsync) { RTNAME(ExecuteCommandLine) (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get()); - CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404); - CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 0); + CheckDescriptorEqInt(exitStat.get(), 404); + CheckDescriptorEqInt(cmdStat.get(), 0); CheckDescriptorEqStr(cmdMsg.get(), "No change"); } @@ -400,7 +400,7 @@ TEST_F(ZeroArguments, ECLInvalidCommandParentNotTerminatedAsync) { EXPECT_NO_FATAL_FAILURE(RTNAME(ExecuteCommandLine)( *command.get(), wait, exitStat.get(), nullptr, cmdMsg.get())); - CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 404); + CheckDescriptorEqInt(exitStat.get(), 404); CheckDescriptorEqStr(cmdMsg.get(), "No change"); } >From a157933536c109fc161fff074fcd4d2fbefc7dc4 Mon Sep 17 00:00:00 2001 From: Yi Wu <yi....@arm.com> Date: Wed, 17 Jan 2024 17:05:14 +0000 Subject: [PATCH 18/18] test fixes --- flang/test/Lower/Intrinsics/system-optional.f90 | 3 +-- flang/test/Lower/Intrinsics/system.f90 | 8 +++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/flang/test/Lower/Intrinsics/system-optional.f90 b/flang/test/Lower/Intrinsics/system-optional.f90 index 2fbdfe4b9f5775..e30549bf95a8df 100644 --- a/flang/test/Lower/Intrinsics/system-optional.f90 +++ b/flang/test/Lower/Intrinsics/system-optional.f90 @@ -26,8 +26,7 @@ subroutine all_args(command, exitstat) ! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> ! CHECK-NEXT: %[[exitstat:.*]] = fir.convert %[[exitstatRealBox]] : (!fir.box<i32>) -> !fir.box<none> ! CHECK-NEXT: %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> -! CHECK-NEXT: %[[VAL_15:.*]] = fir.convert %[[VAL_11:.*]] : (!fir.ref<!fir.char<1,82>>) -> !fir.ref<i8> -! CHECK-NEXT: %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK: %[[VAL_16:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_15:.*]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } diff --git a/flang/test/Lower/Intrinsics/system.f90 b/flang/test/Lower/Intrinsics/system.f90 index 37034bbc0c4f84..a1686fdea7d3ab 100644 --- a/flang/test/Lower/Intrinsics/system.f90 +++ b/flang/test/Lower/Intrinsics/system.f90 @@ -22,8 +22,7 @@ subroutine all_args(command, exitstat) ! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> ! CHECK-NEXT: %[[exitstat:.*]] = fir.convert %[[exitstatBox]] : (!fir.box<i32>) -> !fir.box<none> ! CHECK-NEXT: %[[cmdstat:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> -! CHECK-NEXT: %[[VAL_12:.*]] = fir.convert %[[VAL_8:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> -! CHECK-NEXT: %[[VAL_13:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_12]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK: %[[VAL_13:.*]] = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[exitstat]], %[[cmdstat]], %[[absentBox]], %[[VAL_12:.*]], %[[c9_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } end subroutine all_args @@ -43,11 +42,10 @@ subroutine only_command(command) ! CHECK-NEXT: fir.store %[[c0_i2:.*]] to %[[cmdstatVal]] : !fir.ref<i2> ! CHECK-NEXT: %[[cmdstatBox:.*]] = fir.embox %[[cmdstatVal]] : (!fir.ref<i2>) -> !fir.box<i2> ! CHECK-NEXT: %[[absentBox2:.*]] = fir.absent !fir.box<none> -! CHECK: %[[c35_i32:.*]] = arith.constant 35 : i32 +! CHECK: %[[c34_i32:.*]] = arith.constant 34 : i32 ! CHECK-NEXT: %[[command:.*]] = fir.convert %[[commandBox]] : (!fir.box<!fir.char<1,?>>) -> !fir.box<none> ! CHECK-NEXT: %[[cmdstst:.*]] = fir.convert %[[cmdstatBox]] : (!fir.box<i2>) -> !fir.box<none> -! CHECK-NEXT: %[[VAL_10:.*]]0 = fir.convert %[[VAL_7:.*]] : (!fir.ref<!fir.char<1,73>>) -> !fir.ref<i8> -! CHECK-NEXT: %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10]]0, %[[c35_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none +! CHECK: %[[VAL_11:.*]]1 = fir.call @_FortranAExecuteCommandLine(%[[command]], %[[true]], %[[absentBox]], %[[cmdstst]], %[[absentBox2]], %[[VAL_10:.*]]0, %[[c34_i32]]) fastmath<contract> : (!fir.box<none>, i1, !fir.box<none>, !fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none ! CHECK-NEXT: return ! CHECK-NEXT: } end subroutine only_command _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits