This is an archive of the discontinued LLVM Phabricator instance.

[flang] Define the runtime API for CPU_TIME
ClosedPublic

Authored by rovka on Jun 7 2021, 5:50 AM.

Details

Summary

CPU_TIME takes a single real scalar INTENT(OUT) argument. We can
therefore return a double and let lowering handle casting that to the
precision used for the default real kind.

Diff Detail

Event Timeline

rovka created this revision.Jun 7 2021, 5:50 AM
rovka requested review of this revision.Jun 7 2021, 5:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2021, 5:50 AM
rovka added inline comments.Jun 7 2021, 5:53 AM
flang/runtime/time-intrinsic.h
22

IIUC, the default real kind is usually 4, but can be set to 8 with -fdefault-real-8.

Should we have another overload for CppTypeFor<TypeCategory::Real, 4>, or can we assume that lowering can deal with downcasting if the default kind is 4?

schweitz added inline comments.
flang/runtime/time-intrinsic.h
22

Lowering handles cases which require floating-point format conversions when calling the runtime. That said, the runtime can (and often does) provide multiple interfaces, which may eliminate the necessity of such conversions.

klausler added inline comments.Jun 7 2021, 9:21 AM
flang/runtime/time-intrinsic.h
13

I would consider putting this into the header file with miscellaneous intrinsics... were it not for the problem of SYSTEM_CLOCK (16.9.186), which will also need a home, and whose API might be messy. DATE_AND_TIME will also need to go here.

22

Lowering can handle data conversions for value arguments and for function results, but reference arguments like CPU_TIME's are somewhat more problematic. CPU_TIME (16.9.57) is a subroutine, not a function. But since its sole argument is INTENT(OUT), the API could be a function returning a C double, and lowering could take care of converting and assigning the result.

23

These source position arguments are needed only for runtime APIs that can encounter errors. CPU_TIME probably can't.

rovka added inline comments.Jun 8 2021, 12:52 AM
flang/runtime/time-intrinsic.h
13

Yes, my intention was to add SYSTEM_CLOCK here as well. As you point out, DATE_AND_TIME also fits.

22

Ok, for the sake of simplicity we can go with returning a double (although that trick won't work for SYSTEM_CLOCK, where we need 3 output parameters; but that's a different patch).

23

Ok, removing them.

rovka updated this revision to Diff 350520.Jun 8 2021, 12:53 AM
rovka edited the summary of this revision. (Show Details)

Addressed review comments.

jeanPerier accepted this revision.Jun 8 2021, 2:00 AM

LGTM

flang/runtime/time-intrinsic.h
13

FYI, there is an old draft of DATE_AND_TIME in fir-dev: https://github.com/flang-compiler/f18-llvm-project/blob/fir-dev/flang/runtime/clock.h and https://github.com/flang-compiler/f18-llvm-project/blob/fir-dev/flang/runtime/clock.cpp.
It's missing support for the VALUES argument and lack windows support. I'll upstream it in these new file when your patch is in.
It is the only intrinsic for which something was done in fir-dev only.

This revision is now accepted and ready to land.Jun 8 2021, 2:00 AM
This revision was landed with ongoing or failed builds.Jun 9 2021, 1:33 AM
This revision was automatically updated to reflect the committed changes.