Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang] Add ETIME runtime and lowering intrinsics implementation #90578

Merged
merged 14 commits into from
May 16, 2024

Conversation

JumpMasterJJ
Copy link
Member

@JumpMasterJJ JumpMasterJJ commented Apr 30, 2024

This patch add support of intrinsics GNU extension ETIME #84205. Some usage info and example has been added to flang/docs/Intrinsics.md. The patch contains both the lowering and the runtime code and works on both Windows and Linux.

System Implmentation
Windows GetProcessTimes
Linux times

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added flang:runtime flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Apr 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 30, 2024

@llvm/pr-subscribers-flang-runtime

@llvm/pr-subscribers-flang-semantics

Author: jiajie zhang (JumpMasterJJ)

Changes

This patch add support of intrinsics GNU extension ETIME. Some usage info and example has been added to flang/docs/Intrinsics.md. The patch contains both the lowering and the runtime code and works on both Windows and Linux.

System Implmentation
Windows GetProcessTimes
Linux times

Full diff: https://github.com/llvm/llvm-project/pull/90578.diff

11 Files Affected:

  • (modified) flang/docs/Intrinsics.md (+51)
  • (modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+1)
  • (modified) flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h (+2)
  • (modified) flang/include/flang/Runtime/time-intrinsic.h (+3)
  • (modified) flang/lib/Evaluate/intrinsics.cpp (+10-3)
  • (modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+21)
  • (modified) flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp (+14)
  • (modified) flang/runtime/time-intrinsic.cpp (+69-1)
  • (modified) flang/runtime/tools.h (+9)
  • (added) flang/test/Lower/Intrinsics/etime.f90 (+21)
  • (added) flang/test/Semantics/etime.f90 (+21)
diff --git a/flang/docs/Intrinsics.md b/flang/docs/Intrinsics.md
index 848619cb65d909..46889bd1415226 100644
--- a/flang/docs/Intrinsics.md
+++ b/flang/docs/Intrinsics.md
@@ -916,3 +916,54 @@ used in constant expressions have currently no folding support at all.
   - If a condition occurs that would assign a nonzero value to `CMDSTAT` but the `CMDSTAT` variable is not present, error termination is initiated.
     - On POSIX-compatible systems, the child process (async process) will be terminated with no effect on the parent process (continues).
     - On Windows, error termination is not initiated.
+
+### Non-Standard Intrinsics: ETIME
+
+#### Description
+`ETIME(VALUES, TIME)` returns the number of seconds of runtime since the start of the process’s execution in *TIME*. *VALUES* returns the user and system components of this time in `VALUES(1)` and `VALUES(2)` respectively. *TIME* is equal to `VALUES(1) + VALUES(2)`.
+
+On some systems, the underlying timings are represented using types with sufficiently small limits that overflows (wrap around) are possible, such as 32-bit types. Therefore, the values returned by this intrinsic might be, or become, negative, or numerically less than previous values, during a single run of the compiled program.
+
+This intrinsic is provided in subroutine forms only.
+
+*VALUES* and *TIME* are `INTENT(OUT)` and provide the following:
+
+
+|               |                                   |
+|---------------|-----------------------------------|
+| `VALUES(1)`   | User time in seconds.             |
+| `VALUES(2)`   | System time in seconds.           |
+| `TIME`        | Run time since start in seconds.  |
+
+#### Usage and Info
+
+- **Standard:** GNU extension
+- **Class:** Subroutine
+- **Syntax:** `CALL ETIME(VALUES, TIME)`
+- **Arguments:**
+
+| Argument   | Description                                                           |
+|------------|-----------------------------------------------------------------------|
+| `VALUES`   | The type shall be REAL(4), DIMENSION(2).                              |
+| `TIME`     | The type shall be REAL(4).                                            |
+
+#### Example
+
+```Fortran
+program test_etime
+    integer(8) :: i, j
+    real, dimension(2) :: tarray
+    real :: result
+    call ETIME(tarray, result)
+    print *, result
+    print *, tarray(1)
+    print *, tarray(2)   
+    do i=1,100000000    ! Just a delay
+        j = i * i - i
+    end do
+    call ETIME(tarray, result)
+    print *, result
+    print *, tarray(1)
+    print *, tarray(2)
+end program test_etime
+```
\ No newline at end of file
diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index 6927488517e63b..5c385c04ac3e9d 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -222,6 +222,7 @@ struct IntrinsicLibrary {
   fir::ExtendedValue genEoshift(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
   void genExit(llvm::ArrayRef<fir::ExtendedValue>);
   void genExecuteCommandLine(mlir::ArrayRef<fir::ExtendedValue> args);
+  void genEtime(mlir::ArrayRef<fir::ExtendedValue> args);
   mlir::Value genExponent(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genExtendsTypeOf(mlir::Type,
                                       llvm::ArrayRef<fir::ExtendedValue>);
diff --git a/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h b/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
index 737c631e45c1f6..7497a4bc35646f 100644
--- a/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
+++ b/flang/include/flang/Optimizer/Builder/Runtime/Intrinsics.h
@@ -44,6 +44,8 @@ void genDateAndTime(fir::FirOpBuilder &, mlir::Location,
                     std::optional<fir::CharBoxValue> date,
                     std::optional<fir::CharBoxValue> time,
                     std::optional<fir::CharBoxValue> zone, mlir::Value values);
+void genEtime(fir::FirOpBuilder &builder, mlir::Location loc,
+              mlir::Value values, mlir::Value time);
 
 void genRandomInit(fir::FirOpBuilder &, mlir::Location, mlir::Value repeatable,
                    mlir::Value imageDistinct);
diff --git a/flang/include/flang/Runtime/time-intrinsic.h b/flang/include/flang/Runtime/time-intrinsic.h
index 650c02436ee49e..80490a17e45597 100644
--- a/flang/include/flang/Runtime/time-intrinsic.h
+++ b/flang/include/flang/Runtime/time-intrinsic.h
@@ -43,6 +43,9 @@ void RTNAME(DateAndTime)(char *date, std::size_t dateChars, char *time,
     const char *source = nullptr, int line = 0,
     const Descriptor *values = nullptr);
 
+void RTNAME(Etime)(const Descriptor *values, const Descriptor *time,
+    const char *sourceFile, int line);
+
 } // extern "C"
 } // namespace Fortran::runtime
 #endif // FORTRAN_RUNTIME_TIME_INTRINSIC_H_
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index f07f94b1a022c9..b77df3df756256 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -1340,6 +1340,12 @@ static const IntrinsicInterface intrinsicSubroutine[]{
             {"values", AnyInt, Rank::vector, Optionality::optional,
                 common::Intent::Out}},
         {}, Rank::elemental, IntrinsicClass::impureSubroutine},
+    {"etime",
+        {{"values", TypePattern{RealType, KindCode::exactKind, 4}, Rank::vector,
+             Optionality::required, common::Intent::Out},
+            {"time", TypePattern{RealType, KindCode::exactKind, 4},
+                Rank::scalar, Optionality::required, common::Intent::Out}},
+        {}, Rank::elemental, IntrinsicClass::impureSubroutine},
     {"execute_command_line",
         {{"command", DefaultChar, Rank::scalar},
             {"wait", AnyLogical, Rank::scalar, Optionality::optional},
@@ -1939,7 +1945,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) {
@@ -2277,7 +2283,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(
@@ -2355,7 +2362,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);
     }
   }
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 4ee7258004fa74..f0b0b7c74cf5ac 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -222,6 +222,11 @@ static constexpr IntrinsicHandler handlers[]{
        {"boundary", asBox, handleDynamicOptional},
        {"dim", asValue}}},
      /*isElemental=*/false},
+    {"etime",
+     &I::genEtime,
+     {{{"values", asBox},
+       {"time", asBox}}},
+     /*isElemental=*/false},
     {"execute_command_line",
      &I::genExecuteCommandLine,
      {{{"command", asBox},
@@ -3230,6 +3235,22 @@ void IntrinsicLibrary::genExecuteCommandLine(
                                       exitstatBox, cmdstatBox, cmdmsgBox);
 }
 
+// ETIME
+void IntrinsicLibrary::genEtime(
+    llvm::ArrayRef<fir::ExtendedValue> args) {
+  assert(args.size() == 2);
+
+  mlir::Value values = fir::getBase(args[0]);
+  mlir::Value time = fir::getBase(args[1]);
+
+  if (!values)
+    fir::emitFatalError(loc, "expected VALUES parameter");
+  if (!time)
+    fir::emitFatalError(loc, "expected TIME parameter");
+
+  fir::runtime::genEtime(builder, loc, values, time);
+}
+
 // EXIT
 void IntrinsicLibrary::genExit(llvm::ArrayRef<fir::ExtendedValue> args) {
   assert(args.size() == 1);
diff --git a/flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp b/flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp
index 57c47da0f3f85c..f1e62484e6502e 100644
--- a/flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp
+++ b/flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp
@@ -106,6 +106,20 @@ void fir::runtime::genDateAndTime(fir::FirOpBuilder &builder,
   builder.create<fir::CallOp>(loc, callee, args);
 }
 
+void fir::runtime::genEtime(fir::FirOpBuilder &builder, mlir::Location loc,
+                            mlir::Value values, mlir::Value time) {
+  auto runtimeFunc = fir::runtime::getRuntimeFunc<mkRTKey(Etime)>(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, values, time, sourceFile, sourceLine);
+  builder.create<fir::CallOp>(loc, runtimeFunc, args);
+}
+
 void fir::runtime::genRandomInit(fir::FirOpBuilder &builder, mlir::Location loc,
                                  mlir::Value repeatable,
                                  mlir::Value imageDistinct) {
diff --git a/flang/runtime/time-intrinsic.cpp b/flang/runtime/time-intrinsic.cpp
index 68d63253139f18..93b782693c4cc9 100644
--- a/flang/runtime/time-intrinsic.cpp
+++ b/flang/runtime/time-intrinsic.cpp
@@ -19,8 +19,12 @@
 #include <cstdlib>
 #include <cstring>
 #include <ctime>
-#ifndef _WIN32
+#ifdef _WIN32
+#include "flang/Common/windows-include.h"
+#else
 #include <sys/time.h> // gettimeofday
+#include <sys/times.h>
+#include <unistd.h>
 #endif
 
 // CPU_TIME (Fortran 2018 16.9.57)
@@ -370,5 +374,69 @@ void RTNAME(DateAndTime)(char *date, std::size_t dateChars, char *time,
       terminator, date, dateChars, time, timeChars, zone, zoneChars, values);
 }
 
+void RTNAME(Etime)(const Descriptor *values, const Descriptor *time,
+    const char *sourceFile, int line) {
+  Fortran::runtime::Terminator terminator{sourceFile, line};
+
+  double usrTime = -1.0, sysTime = -1.0, realTime = -1.0;
+
+#ifdef _WIN32
+  FILETIME creationTime;
+  FILETIME exitTime;
+  FILETIME kernelTime;
+  FILETIME userTime;
+
+  if (GetProcessTimes(GetCurrentProcess(), &creationTime, &exitTime,
+          &kernelTime, &userTime) != -1) {
+    ULARGE_INTEGER userSystemTime;
+    ULARGE_INTEGER kernelSystemTime;
+
+    memcpy(&userSystemTime, &userTime, sizeof(FILETIME));
+    memcpy(&kernelSystemTime, &kernelTime, sizeof(FILETIME));
+
+    usrTime = double(userSystemTime.QuadPart / 10000);
+    sysTime = double(kernelSystemTime.QuadPart / 10000);
+    realTime = usrTime + sysTime;
+  }
+#else
+  struct tms tms;
+  if (times(&tms) != -1) {
+    usrTime = (double)(tms.tms_utime) / sysconf(_SC_CLK_TCK);
+    sysTime = (double)(tms.tms_stime) / sysconf(_SC_CLK_TCK);
+    realTime = usrTime + sysTime;
+  }
+#endif
+
+  if (values) {
+    auto typeCode{values->type().GetCategoryAndKind()};
+    // ETIME values argument must have decimal range == 2.
+    RUNTIME_CHECK(terminator,
+        values->rank() == 1 && values->GetDimension(0).Extent() == 2 &&
+            typeCode && typeCode->first == Fortran::common::TypeCategory::Real);
+    // Only accept KIND=4 here.
+    int kind{typeCode->second};
+    RUNTIME_CHECK(terminator, kind == 4);
+
+    ApplyFloatingPointKind<StoreFloatingPointAt, void>(
+        kind, terminator, *values, /* atIndex = */ 0, usrTime);
+    ApplyFloatingPointKind<StoreFloatingPointAt, void>(
+        kind, terminator, *values, /* atIndex = */ 1, sysTime);
+  }
+
+  if (time) {
+    auto typeCode{time->type().GetCategoryAndKind()};
+    // ETIME time argument must have decimal range == 0.
+    RUNTIME_CHECK(terminator,
+        time->rank() == 0 && typeCode &&
+            typeCode->first == Fortran::common::TypeCategory::Real);
+    // Only accept KIND=4 here.
+    int kind{typeCode->second};
+    RUNTIME_CHECK(terminator, kind == 4);
+
+    ApplyFloatingPointKind<StoreFloatingPointAt, void>(
+        kind, terminator, *time, /* atIndex = */ 0, realTime);
+  }
+}
+
 } // extern "C"
 } // namespace Fortran::runtime
diff --git a/flang/runtime/tools.h b/flang/runtime/tools.h
index 52049c511f13ed..dc12e5c4533e2e 100644
--- a/flang/runtime/tools.h
+++ b/flang/runtime/tools.h
@@ -99,6 +99,15 @@ template <int KIND> struct StoreIntegerAt {
   }
 };
 
+// Helper to store floating value in result[at].
+template <int KIND> struct StoreFloatingPointAt {
+  RT_API_ATTRS void operator()(const Fortran::runtime::Descriptor &result,
+      std::size_t at, std::double_t value) const {
+    *result.ZeroBasedIndexedElement<Fortran::runtime::CppTypeFor<
+        Fortran::common::TypeCategory::Real, KIND>>(at) = value;
+  }
+};
+
 // Validate a KIND= argument
 RT_API_ATTRS void CheckIntegerKind(
     Terminator &, int kind, const char *intrinsic);
diff --git a/flang/test/Lower/Intrinsics/etime.f90 b/flang/test/Lower/Intrinsics/etime.f90
new file mode 100644
index 00000000000000..e5e7984a340caa
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/etime.f90
@@ -0,0 +1,21 @@
+! RUN: bbc -emit-fir %s -o - | FileCheck %s
+
+! CHECK-LABEL: func.func @_QPetime_test(
+! CHECK-SAME: %[[valuesArg:.*]]: !fir.ref<!fir.array<2xf32>> {fir.bindc_name = "values"},
+! CHECK-SAME: %[[timeArg:.*]]: !fir.ref<f32> {fir.bindc_name = "time"}) {
+subroutine etime_test(values, time)
+  REAL(4), DIMENSION(2) :: values
+  REAL(4) :: time
+  call etime(values, time)
+  ! CHECK-NEXT:        %[[c9:.*]] = arith.constant 9 : i32
+  ! CHECK-NEXT:        %[[c2:.*]] = arith.constant 2 : index
+  ! CHECK-NEXT:        %[[timeDeclare:.*]] = fir.declare %[[timeArg]] {uniq_name = "_QFetime_testEtime"} : (!fir.ref<f32>) -> !fir.ref<f32>
+  ! CHECK-NEXT:        %[[shape:.*]] = fir.shape %[[c2]] : (index) -> !fir.shape<1>
+  ! CHECK-NEXT:        %[[valuesDeclare:.*]] = fir.declare %[[valuesArg]](%[[shape]]) {uniq_name = "_QFetime_testEvalues"} : (!fir.ref<!fir.array<2xf32>>, !fir.shape<1>) -> !fir.ref<!fir.array<2xf32>>
+  ! CHECK-NEXT:        %[[valuesBox:.*]] = fir.embox %[[valuesDeclare]](%[[shape]]) : (!fir.ref<!fir.array<2xf32>>, !fir.shape<1>) -> !fir.box<!fir.array<2xf32>>
+  ! CHECK-NEXT:        %[[timeBox:.*]] = fir.embox %[[timeDeclare]] : (!fir.ref<f32>) -> !fir.box<f32>
+  ! CHECK:             %[[values:.*]] = fir.convert %[[valuesBox]] : (!fir.box<!fir.array<2xf32>>) -> !fir.box<none>
+  ! CHECK:             %[[time:.*]] = fir.convert %[[timeBox]] : (!fir.box<f32>) -> !fir.box<none>
+  ! CHECK:             %[[VAL_9:.*]] = fir.call @_FortranAEtime(%[[values]], %[[time]], %[[VAL_7:.*]], %[[c9]]) fastmath<contract> : (!fir.box<none>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+  ! CHECK-NEXT:        return
+end subroutine etime_test
\ No newline at end of file
diff --git a/flang/test/Semantics/etime.f90 b/flang/test/Semantics/etime.f90
new file mode 100644
index 00000000000000..176b63b2a576dc
--- /dev/null
+++ b/flang/test/Semantics/etime.f90
@@ -0,0 +1,21 @@
+! RUN: %python %S/test_errors.py %s %flang_fc1 -pedantic
+! Tests for the ETIME intrinsics
+
+subroutine bad_kind_error(values, time)
+  REAL(KIND=8), DIMENSION(2) :: values
+  REAL(KIND=8) :: time
+  !ERROR: Actual argument for 'values=' has bad type or kind 'REAL(8)'
+  call etime(values, time)
+end subroutine bad_kind_error
+  
+subroutine bad_args_error(values)
+  REAL(KIND=4), DIMENSION(2) :: values
+  !ERROR: missing mandatory 'time=' argument
+  call etime(values)
+end subroutine bad_args_error
+
+subroutine good_kind_equal(values, time)
+  REAL(KIND=4), DIMENSION(2) :: values
+  REAL(KIND=4) :: time
+  call etime(values, time)
+end subroutine good_kind_equal
\ No newline at end of file

Copy link

github-actions bot commented Apr 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@mjklemm mjklemm requested review from tblah and klausler April 30, 2024 14:35
Copy link
Contributor

@klausler klausler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

etime needs to be both a function and a subroutine.

flang/lib/Evaluate/intrinsics.cpp Show resolved Hide resolved
Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to Flang! Thanks for adding this.

{"etime",
&I::genEtime,
{{{"values", asBox},
{"time", asBox}}},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is time as a box? I guess this should work but passing by reference is simpler and avoids creating a box on the fly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about it. I can try it later.

flang/runtime/time-intrinsic.cpp Outdated Show resolved Hide resolved
flang/runtime/time-intrinsic.cpp Outdated Show resolved Hide resolved
flang/test/Semantics/etime.f90 Show resolved Hide resolved
mlir::Value time = fir::getBase(args[1]);

if (!values)
fir::emitFatalError(loc, "expected VALUES parameter");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortran uses "argument" instead of "parameter".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, but I was referring to the code here. So, should this also be "arguments"?

if (!command)
fir::emitFatalError(loc, "expected COMMAND parameter");

if (!number)
fir::emitFatalError(loc, "expected NUMBER parameter");

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that needs to be changed as well. Perhaps we can do it in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's keep this PR focused on the etime intrinsic. I will make a PR to clean this up (and also fix up the elemental issue with most of the intrinsics, as @klausler as noted correctly).

if (!values)
fir::emitFatalError(loc, "expected VALUES parameter");
if (!time)
fir::emitFatalError(loc, "expected TIME parameter");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fortran uses "argument" instead of "parameter".

@mjklemm
Copy link
Contributor

mjklemm commented May 1, 2024

etime needs to be both a function and a subroutine.

Yes. Any recommendation how this can be achieved? I was trying to add etime to the function definitions and the subroutine definitions, but then code-gen handler is not being picked up.

@klausler
Copy link
Contributor

klausler commented May 1, 2024

etime needs to be both a function and a subroutine.

Yes. Any recommendation how this can be achieved? I was trying to add etime to the function definitions and the subroutine definitions, but then code-gen handler is not being picked up.

I don't understand that last part. Does it mean that lowering needs distinct names for the function and the subroutine forms? That seems likely, and one can do that by defining a distinct name in the function table (say __builtin_etime_func) and then defining etime as an alias for it in genericAlias.

@mjklemm
Copy link
Contributor

mjklemm commented May 7, 2024

@JumpMasterJJ Please let me know if you need any help with the pull request to complete the open items. I can try to help if you need me.

@JumpMasterJJ
Copy link
Member Author

JumpMasterJJ commented May 7, 2024

etime needs to be both a function and a subroutine.

Yes. Any recommendation how this can be achieved? I was trying to add etime to the function definitions and the subroutine definitions, but then code-gen handler is not being picked up.

I am trying to declare a DualGenerator to solve all intrinsics with both function and subroutine forms. It looks like this:

// in IntrinsicCall.h

/// Define the different FIR generators that can be mapped to intrinsic to
/// generate the related code.
using ElementalGenerator = decltype(&IntrinsicLibrary::genAbs);
using ExtendedGenerator = decltype(&IntrinsicLibrary::genLenTrim);
using SubroutineGenerator = decltype(&IntrinsicLibrary::genDateAndTime);
// TODO: etime
/// The generator for intrinsic that has both function and subroutine form.
using DualGenerator = decltype(&IntrinsicLibrary::genEtime);
using Generator =
    std::variant<ElementalGenerator, ExtendedGenerator, SubroutineGenerator, DualGenerator>;
// in IntrinsicCall.cpp

// TODO: etime
static fir::ExtendedValue
invokeHandler(IntrinsicLibrary::DualGenerator generator,
              const IntrinsicHandler &handler,
              std::optional<mlir::Type> resultType,
              llvm::ArrayRef<fir::ExtendedValue> args, bool outline,
              IntrinsicLibrary &lib) {
  if (handler.isElemental)
    return lib.genElementalCall(generator, handler.name, mlir::Type{}, args,
                                outline);
  if (outline)
    return lib.outlineInExtendedWrapper(generator, handler.name, resultType,
                                        args);

  if (resultType.has_value()) {
    // function form
    return std::invoke(generator, lib, *resultType, args);
  } else {
    // subroutine form
    std::invoke(generator, lib, args);
  }
  return mlir::Value{};
}

How about this plan? Do you have any comments

@JumpMasterJJ
Copy link
Member Author

@JumpMasterJJ Please let me know if you need any help with the pull request to complete the open items. I can try to help if you need me.

Please let me know if you need any help with the pull request to complete the open items. I can try to help if you need me.

Thank you for your offer. I'll ask for your help if I need more assistance. It's great to know you're there to help.

@mjklemm
Copy link
Contributor

mjklemm commented May 10, 2024

How about this plan? Do you have any comments

I think introducing a new category will be fine. I'm actually not sure how many of these intrinsics exist (I haven't checked the GFortran docs). So, maybe the alias route might be the easier one for this instance.

@JumpMasterJJ
Copy link
Member Author

I think introducing a new category will be fine. I'm actually not sure how many of these intrinsics exist (I haven't checked the GFortran docs). So, maybe the alias route might be the easier one for this instance.

Actually, Gfortran has a total of 25 such intrinsics. Here are a few examples:
https://gcc.gnu.org/onlinedocs/gfortran/CHDIR.html
https://gcc.gnu.org/onlinedocs/gfortran/FGET.html
https://gcc.gnu.org/onlinedocs/gfortran/HOSTNM.html
...

@JumpMasterJJ
Copy link
Member Author

JumpMasterJJ commented May 12, 2024

@mjklemm @klausler I just updated the function form of etime, is this acceptable to you?

Copy link
Contributor

@mjklemm mjklemm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for updating the PR to also cover the function form of etime. Except for my very tiny nit, I think the PR looks good.

flang/lib/Evaluate/intrinsics.cpp Outdated Show resolved Hide resolved
flang/lib/Evaluate/intrinsics.cpp Show resolved Hide resolved
@JumpMasterJJ JumpMasterJJ merged commit 245b7b6 into llvm:main May 16, 2024
5 checks passed
Copy link

@JumpMasterJJ Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested
by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself.
This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!

joker-eph added a commit that referenced this pull request May 16, 2024
…ion" (#92354)

Reverts #90578

This broke the premerge linux buildbot.
@joker-eph
Copy link
Collaborator

This broke this bot: https://lab.llvm.org/buildbot/#/builders/272/builds/16718

I reverted in: #92354

mub-at-arm pushed a commit to mub-at-arm/llvm-project that referenced this pull request May 16, 2024
…m#90578)

This patch add support of intrinsics GNU extension ETIME
llvm#84205. Some usage info and
example has been added to `flang/docs/Intrinsics.md`. The patch contains
both the lowering and the runtime code and works on both Windows and
Linux.


|   System  |   Implmentation  |
|-----------|--------------------|
| Windows| GetProcessTimes |
| Linux      |times                     |
mub-at-arm pushed a commit to mub-at-arm/llvm-project that referenced this pull request May 16, 2024
JumpMasterJJ added a commit that referenced this pull request May 17, 2024
…ion (#92571)

This is same as #90578 with an
added fix. This PR updated tests of etime intrinsic due to Lowering
changes for assigning dummy_scope to hlfir.declare. Referring to
#92472 and
#90989
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:fir-hlfir flang:runtime flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants