[flang] Register and lower SECNDS (stubbed implementation) #151878

mlir-maiden · 2025-08-03T18:13:26Z

This patch registers and lowers the GNU extension intrinsic SECNDS in Flang.

Registration and lowering are wired through the intrinsic infrastructure.
genSecnds() currently emits a TODO fatal error, marking it as unimplemented.
Includes an XFAIL test to exercise the lowering path and reflect current status.

Fixes #58728

CC @eugeneepshteyn @klausler

github-actions · 2025-08-03T18:13:44Z

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 · 2025-08-03T18:14:12Z

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-semantics

Author: Šárka Holendová (mlir-maiden)

Changes

This patch registers and lowers the GNU extension intrinsic SECNDS in Flang.

Registration and lowering are wired through the intrinsic infrastructure.
genSecnds() currently emits a TODO fatal error, marking it as unimplemented.
Includes an XFAIL test to exercise the lowering path and reflect current status.

TODOs:

Implement runtime behavior for SECNDS

CC @eugeneepshteyn @klausler

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

4 Files Affected:

(modified) flang/include/flang/Optimizer/Builder/IntrinsicCall.h (+2)
(modified) flang/lib/Evaluate/intrinsics.cpp (+5)
(modified) flang/lib/Optimizer/Builder/IntrinsicCall.cpp (+12)
(added) flang/test/Lower/Intrinsics/secnds.f90 (+12)

diff --git a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
index d38c5b6d09a82..62ecd6c8a53f1 100644
--- a/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
+++ b/flang/include/flang/Optimizer/Builder/IntrinsicCall.h
@@ -401,6 +401,8 @@ struct IntrinsicLibrary {
                                    llvm::ArrayRef<fir::ExtendedValue>);
   mlir::Value genScale(mlir::Type, llvm::ArrayRef<mlir::Value>);
   fir::ExtendedValue genScan(mlir::Type, llvm::ArrayRef<fir::ExtendedValue>);
+  fir::ExtendedValue genSecnds(mlir::Type resultType,
+                               llvm::ArrayRef<fir::ExtendedValue> args);
   fir::ExtendedValue genSecond(std::optional<mlir::Type>,
                                mlir::ArrayRef<fir::ExtendedValue>);
   fir::ExtendedValue genSelectedCharKind(mlir::Type,
diff --git a/flang/lib/Evaluate/intrinsics.cpp b/flang/lib/Evaluate/intrinsics.cpp
index 4773e136c41cb..634154d994c25 100644
--- a/flang/lib/Evaluate/intrinsics.cpp
+++ b/flang/lib/Evaluate/intrinsics.cpp
@@ -918,6 +918,11 @@ static const IntrinsicInterface genericIntrinsicFunction[]{
             {"back", AnyLogical, Rank::elemental, Optionality::optional},
             DefaultingKIND},
         KINDInt},
+    {"secnds",
+        {{"x", TypePattern{RealType, KindCode::exactKind, 4}, Rank::scalar,
+            Optionality::required, common::Intent::In}},
+        TypePattern{RealType, KindCode::exactKind, 4}, Rank::scalar,
+        IntrinsicClass::impureSubroutine},
     {"second", {}, DefaultReal, Rank::scalar},
     {"selected_char_kind", {{"name", DefaultChar, Rank::scalar}}, DefaultInt,
         Rank::scalar, IntrinsicClass::transformationalFunction},
diff --git a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
index 8d0a511744e25..9b037cc66c59e 100644
--- a/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
+++ b/flang/lib/Optimizer/Builder/IntrinsicCall.cpp
@@ -852,6 +852,10 @@ static constexpr IntrinsicHandler handlers[]{
        {"back", asValue, handleDynamicOptional},
        {"kind", asValue}}},
      /*isElemental=*/true},
+    {"secnds",
+     &I::genSecnds,
+     {{{"x", asValue}}},
+     /*isElemental=*/false},
     {"second",
      &I::genSecond,
      {{{"time", asAddr}}},
@@ -7722,6 +7726,14 @@ IntrinsicLibrary::genScan(mlir::Type resultType,
   return readAndAddCleanUp(resultMutableBox, resultType, "SCAN");
 }
 
+// SECNDS
+// Lowering is registered, runtime not yet implemented
+fir::ExtendedValue
+IntrinsicLibrary::genSecnds(mlir::Type resultType,
+                            llvm::ArrayRef<fir::ExtendedValue> args) {
+  TODO(loc, "not yet implemented: SECNDS");
+}
+
 // SECOND
 fir::ExtendedValue
 IntrinsicLibrary::genSecond(std::optional<mlir::Type> resultType,
diff --git a/flang/test/Lower/Intrinsics/secnds.f90 b/flang/test/Lower/Intrinsics/secnds.f90
new file mode 100644
index 0000000000000..105e93ad9301e
--- /dev/null
+++ b/flang/test/Lower/Intrinsics/secnds.f90
@@ -0,0 +1,12 @@
+!---------------------------------------------------------------------
+! RUN: %flang_fc1 -emit-fir %s -o - 2>&1 | FileCheck %s
+! XFAIL: *
+!---------------------------------------------------------------------
+
+program test_secnds
+  real :: x
+  x = secnds(1.0)
+end program
+
+! CHECK: not yet implemented: SECNDS
+

eugeneepshteyn · 2025-08-03T21:14:29Z

flang/lib/Optimizer/Builder/IntrinsicCall.cpp

Hello, thank you for the patch!

I happen to know how to add secnds() to the runtime part, so I can quickly do that part.

genSecnds() though, is not part of runtime, it's part of flang compiler that generates MLIR/FIR/HLFIR code to set up the runtime call. Would you like to try to do that? That's probably going to be similar (but easier) to genEtime(), where the input arg is always one REAL.

Hello Eugene,
Thank you for the review and the offer to handle the runtime. I’ll gladly take you up on that. Since the registration and lowering are confirmed, I’d like to take it a step further and wire genSecnds() to a stubbed RTNAME(Secnds) in time-intrinsic.cpp, so the full path is in place. I know how to do that. I’m on it and will follow up with a patch shortly.

Best,
Sarka

eugeneepshteyn · 2025-08-03T21:47:22Z

Also, could you please document this new intrinsic in https://github.com/llvm/llvm-project/blob/main/flang/docs/Intrinsics.md ?

klausler · 2025-08-04T15:30:23Z

flang/lib/Evaluate/intrinsics.cpp

Are you sure that the reference argument is always REAL(4), or is it the default REAL kind? Are the semantics in GNU Fortran affected when the default REAL kind is changed on the command line?

Optionality::required and Intent::In are the default settings, so you don't need to specify them again here.

Thank you, Peter. And nice to e-meet you.
I tested this with gfortran using -fdefault-real-8 to check whether SECNDS follows the default REAL kind. It turns out SECNDS requires REAL(4) explicitly. When compiled with
-fdefault-real-8, it produces a compile-time error:
'x' argument of 'secnds' intrinsic ... must be of kind 4

So the use of KindCode::exactKind, 4 matches gfortran's behavior.

Also noted on Optionality::required and Intent::In. I will drop those for clarity.

klausler · 2025-08-04T15:31:30Z

flang/lib/Evaluate/intrinsics.cpp

I think that SECNDS is a function.

I went back and forth on the classification quite a bit.
SECNDS is a non-standard intrinsic, and GNU Fortran documents it simply as a function. But when mapping it to Flang’s IntrinsicClass, none of the available categories seem to fit perfectly. It is impure (as confirmed by GNU rejecting its use in PURE procedures), and while it has a scalar argument and a scalar return value, it cannot be classified as an elementalFunction. According to ISO/IEC 1539-1:2023 §15.9.1, “An elemental subprogram is a pure subprogram unless it has the prefix-spec IMPURE.” Since SECNDS has no ELEMENTAL or IMPURE prefix and is not pure, it doesn't qualify as elemental. I am assuming here that GNU Fortran would explicitly document those attributes if they were part of the intrinsic’s definition. ETIME is another non-standard intrinsic and is left unclassified in Flang. Based on that precedent and the lack of a defined category for impure functions, it seems best to leave SECNDS unclassified as well. I am open to suggestions if there is a more appropriate fit. Thank you :)

Hi Peter, quick follow-up. I realized that leaving intrinsicClass blank defaults to elementalFunction, which would misclassify SECNDS. I also misspoke earlier when I said ETIME was left unclassified. I know it has two entries: one in genericIntrinsicFunction[] (unclassified, so it defaults to elementalFunction), and another in the intrinsicSubroutine table, where it’s correctly marked.
It also raises a question, since ETIME is impure and reads the system clock, should ETIME’s function form really default to elementalFunction? I am not sure what the best fit is for SECNDS. Thank you

eugeneepshteyn · 2025-08-04T20:58:14Z

Runtime PR: #152021

…DS() (#152021) Until the compiler part is fully hooked up via #151878, tested this using `external`: ``` external secnds real s1, s2 s1 = secnds(0.0) print *, "Seconds from midnight:", s1 call sleep(2) s2 = secnds(s1) print *, "Seconds from s1", s2 print *, "Seconds from midnight:", secnds(0.0) end ```

eugeneepshteyn · 2025-08-06T20:14:12Z

@mlir-maiden , I submitted runtime implementation for secnds() upstream. You can now test end-to-end with trying to make the actual function call.

In my implementation I defined runtime wrapper function for secnds() as follows:

float RTNAME(Secnds)(float *refTime, const char *sourceFile, int line) {
  Terminator terminator{sourceFile, line};
  RUNTIME_CHECK(terminator, refTime != nullptr);
  return FORTRAN_PROCEDURE_NAME(secnds)(refTime);
}

Feel free to modify, if that doesn't work for you.

Peter also asked me to make the core implementation of secnds runtime into a template, so that it could be instantiated with double in case we want to also add DSECNDS(). (If you end up implementing DSECNDS(), I recommend you do that as a separate PR.)

…nction SECNDS() (#152021) Until the compiler part is fully hooked up via llvm/llvm-project#151878, tested this using `external`: ``` external secnds real s1, s2 s1 = secnds(0.0) print *, "Seconds from midnight:", s1 call sleep(2) s2 = secnds(s1) print *, "Seconds from s1", s2 print *, "Seconds from midnight:", secnds(0.0) end ```

klausler · 2025-08-06T20:17:31Z

@mlir-maiden , I submitted runtime implementation for secnds() upstream. You can now test end-to-end with trying to make the actual function call.

In my implementation I defined runtime wrapper function for secnds() as follows:
float RTNAME(Secnds)(float *refTime, const char *sourceFile, int line) {
  Terminator terminator{sourceFile, line};
  RUNTIME_CHECK(terminator, refTime != nullptr);
  return FORTRAN_PROCEDURE_NAME(secnds)(refTime);
}
Feel free to modify, if that doesn't work for you.

Peter also asked me to make the core implementation of secnds runtime into a template, so that it could be instantiated with double in case we want to also add DSECNDS(). (If you end up implementing DSECNDS(), I recommend you do that as a separate PR.)

It's SECNDSD and it might be just a PGI extension (also in "classic" flang).

eugeneepshteyn · 2025-08-06T22:49:08Z

It's SECNDSD and it might be just a PGI extension (also in "classic" flang).

Just to close on this, Peter and I confirmed that the extra intrinsic is DSECNDS (and it is a PGI extension).

eugeneepshteyn · 2025-08-06T23:11:50Z

flang-rt/lib/runtime/time-intrinsic.cpp

Note that in Fortran the args are generally passed by reference, hence in my PR this is float *refTime.

This should be the case for routines called by Fortran, but it need not be so for runtime library entry points called only by lowered code, like this.

Roger that, thank you

eugeneepshteyn · 2025-08-06T23:12:18Z

flang/include/flang/Runtime/time-intrinsic.h

float *refTime

This one actually does not need to be a pointer or reference, and it would make lowering a little easier if it wasn't.

eugeneepshteyn · 2025-08-06T23:19:33Z

flang/lib/Optimizer/Builder/Runtime/Intrinsics.cpp

Wouldn't it be something like return fir::CallOp::create(builder, loc, runtimeFunc, args).getResult(0); ? And need to fir::runtime::createArguments() to create args for refTime.

eugeneepshteyn · 2025-08-06T23:21:06Z

flang/lib/Evaluate/intrinsics.cpp

This space between * and arg seems to be wrong. Did you run your changes through clang-format?

Thanks Eugene! I did run clang-format, and you're right it looks like a space between * and arg got introduced here. I likely did that unintentionally. It's interesting that Clang-format did not enforce this spacing style. I will fix that.

Eugene, I tracked this down. On my setup, a full clang-format pass reintroduces two issues in this file: (1) it inserts a space between * and the variable name (one occurrence, as in the screenshot above), and (2) it re-indents ++specIter in roughly three places. To avoid that churn, I pushed a small follow-up commit that hand-fixes just these spots - pointer spacing (no space after *) and the ++specIter indentation. I didn’t run clang-format over the whole file so these lines stay as intended.

eugeneepshteyn · 2025-08-06T23:21:33Z

flang/lib/Evaluate/intrinsics.cpp

Make sure that clang-format is happy with it.

Same here, commented above.

eugeneepshteyn · 2025-08-06T23:22:29Z

flang/test/Lower/Intrinsics/secnds.f90

Now you should be able to check for actual MLIR.

- Updated SECNDS handler to pass `refTime` asAddr instead of asValue, matching the backend runtime contract (`float*`). - Addressed review feedback in the lowering hook: * Built the argument list using fir::runtime::createArguments * Replaced previous call emission with fir::CallOp::create(...) - Updated the regression test for SECNDS lowering to check argument passing, call signature, and return type. - Added documentation for SECNDS to Intrinsics.md, including description, usage, and example.

mlir-maiden

So nice to e-meet you, Peter and Eugene. What an introduction - you won’t believe what happened! I have been quickly replying to your review comments as they came in over the past week (and was so proud of it!), only to find out today that, due to a GitHub review workflow quirk, they were never actually sent. I’m still getting the hang of GitHub, so my apologies.
I’m submitting them now so they’re finally visible. I do want to work on extending DSECNDS, and thank you, Eugene, for the tip+template. It’s very helpful. Glad to finally be “online” with you both now.
@eugeneepshteyn @klausler

mlir-maiden · 2025-08-04T14:03:46Z

flang/lib/Optimizer/Builder/IntrinsicCall.cpp

Hello Eugene,
Thank you for the review and the offer to handle the runtime. I’ll gladly take you up on that. Since the registration and lowering are confirmed, I’d like to take it a step further and wire genSecnds() to a stubbed RTNAME(Secnds) in time-intrinsic.cpp, so the full path is in place. I know how to do that. I’m on it and will follow up with a patch shortly.

Best,
Sarka

mlir-maiden · 2025-08-04T16:45:14Z

flang/lib/Evaluate/intrinsics.cpp

Thank you, Peter. And nice to e-meet you.
I tested this with gfortran using -fdefault-real-8 to check whether SECNDS follows the default REAL kind. It turns out SECNDS requires REAL(4) explicitly. When compiled with
-fdefault-real-8, it produces a compile-time error:
'x' argument of 'secnds' intrinsic ... must be of kind 4

So the use of KindCode::exactKind, 4 matches gfortran's behavior.

Also noted on Optionality::required and Intent::In. I will drop those for clarity.

mlir-maiden · 2025-08-04T19:01:32Z

flang/lib/Evaluate/intrinsics.cpp

I went back and forth on the classification quite a bit.
SECNDS is a non-standard intrinsic, and GNU Fortran documents it simply as a function. But when mapping it to Flang’s IntrinsicClass, none of the available categories seem to fit perfectly. It is impure (as confirmed by GNU rejecting its use in PURE procedures), and while it has a scalar argument and a scalar return value, it cannot be classified as an elementalFunction. According to ISO/IEC 1539-1:2023 §15.9.1, “An elemental subprogram is a pure subprogram unless it has the prefix-spec IMPURE.” Since SECNDS has no ELEMENTAL or IMPURE prefix and is not pure, it doesn't qualify as elemental. I am assuming here that GNU Fortran would explicitly document those attributes if they were part of the intrinsic’s definition. ETIME is another non-standard intrinsic and is left unclassified in Flang. Based on that precedent and the lack of a defined category for impure functions, it seems best to leave SECNDS unclassified as well. I am open to suggestions if there is a more appropriate fit. Thank you :)

mlir-maiden · 2025-08-05T22:57:52Z

flang/lib/Evaluate/intrinsics.cpp

Hi Peter, quick follow-up. I realized that leaving intrinsicClass blank defaults to elementalFunction, which would misclassify SECNDS. I also misspoke earlier when I said ETIME was left unclassified. I know it has two entries: one in genericIntrinsicFunction[] (unclassified, so it defaults to elementalFunction), and another in the intrinsicSubroutine table, where it’s correctly marked.
It also raises a question, since ETIME is impure and reads the system clock, should ETIME’s function form really default to elementalFunction? I am not sure what the best fit is for SECNDS. Thank you

mlir-maiden · 2025-08-07T17:11:22Z

flang/lib/Evaluate/intrinsics.cpp

Thanks Eugene! I did run clang-format, and you're right it looks like a space between * and arg got introduced here. I likely did that unintentionally. It's interesting that Clang-format did not enforce this spacing style. I will fix that.

mlir-maiden · 2025-08-07T17:29:13Z

flang-rt/lib/runtime/time-intrinsic.cpp

Roger that, thank you

mlir-maiden · 2025-08-10T13:28:42Z

flang/lib/Evaluate/intrinsics.cpp

Eugene, I tracked this down. On my setup, a full clang-format pass reintroduces two issues in this file: (1) it inserts a space between * and the variable name (one occurrence, as in the screenshot above), and (2) it re-indents ++specIter in roughly three places. To avoid that churn, I pushed a small follow-up commit that hand-fixes just these spots - pointer spacing (no space after *) and the ++specIter indentation. I didn’t run clang-format over the whole file so these lines stay as intended.

mlir-maiden · 2025-08-10T13:28:56Z

flang/lib/Evaluate/intrinsics.cpp

Same here, commented above.

klausler · 2025-08-11T16:51:46Z

Feel free to extend Fortran::evaluate::IntrinsicClass with a new value impureFunction if that's what makes sense. You'll need to extend the function AcquireIntrinsicProcedureFlags in name resolution to handle it, but it should be straightforward.

mlir-maiden · 2025-08-18T15:57:15Z

@klausler Good idea. I’ll land SECNDS as-is, then follow up with a new PR to add impureFunction and reclassify it.

mlir-maiden · 2025-08-20T18:06:52Z

@klausler @eugeneepshteyn
Hello team, just a gentle nudge on this. I believe the outstanding comments have been addressed, but please let me know if there’s anything further needed on my side. Thank you for your time and review!

eugeneepshteyn · 2025-08-25T17:10:32Z

...I believe the outstanding comments have been addressed, but please let me know if there’s anything further needed on my side.

I checked with Peter and he's ok with this going in. I'm going to check again today to ensure this still works with the latest upstream, and will then approve this PR.

I don't think you have privileges to merge this in, so I'll do the merge myself.

github-actions · 2025-08-26T02:32:04Z

@mlir-maiden 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!

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:semantics labels Aug 3, 2025

eugeneepshteyn reviewed Aug 3, 2025

View reviewed changes

klausler reviewed Aug 4, 2025

View reviewed changes

eugeneepshteyn mentioned this pull request Aug 4, 2025

[flang-rt] Runtime implementation of extended intrinsic function SECNDS() #152021

Merged

mlir-maiden force-pushed the secnds-lowering branch from 9a2a009 to 8c6c34a Compare August 6, 2025 01:42

eugeneepshteyn reviewed Aug 6, 2025

View reviewed changes

mlir-maiden added 2 commits August 7, 2025 15:10

[flang] Register and lower SECNDS (stubbed implementation)

dae2e99

mlir-maiden force-pushed the secnds-lowering branch from 8c6c34a to c787475 Compare August 9, 2025 22:16

Fix pointer spacing and indentation

7de65bd

mlir-maiden commented Aug 10, 2025

View reviewed changes

eugeneepshteyn mentioned this pull request Aug 11, 2025

Add GNU extensions to flang library #58728

Closed

Merge branch 'main' into secnds-lowering

ad2f8a4

eugeneepshteyn merged commit 86bca29 into llvm:main Aug 26, 2025
10 checks passed

[flang] Register and lower SECNDS (stubbed implementation) #151878

[flang] Register and lower SECNDS (stubbed implementation) #151878

Uh oh!

Conversation

mlir-maiden commented Aug 3, 2025 • edited by eugeneepshteyn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Aug 3, 2025

Uh oh!

llvmbot commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODOs:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eugeneepshteyn commented Aug 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eugeneepshteyn commented Aug 4, 2025

Uh oh!

eugeneepshteyn commented Aug 6, 2025

Uh oh!

klausler commented Aug 6, 2025

Uh oh!

eugeneepshteyn commented Aug 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlir-maiden left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mlir-maiden commented Aug 3, 2025 •

edited by eugeneepshteyn

Loading

llvmbot commented Aug 3, 2025 •

edited

Loading