[PATCH] D103805: [flang] Define the runtime API for CPU_TIME

[Diana Picus via Phabricator via llvm-commits]

rovka added inline comments.


================
Comment at: flang/runtime/time-intrinsic.h:12
+
+#ifndef FORTRAN_RUNTIME_TIME_INTRINSIC_H_
+#define FORTRAN_RUNTIME_TIME_INTRINSIC_H_
----------------
klausler wrote:
> 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.
Yes, my intention was to add SYSTEM_CLOCK here as well. As you point out, DATE_AND_TIME also fits.


================
Comment at: flang/runtime/time-intrinsic.h:21
+
+void RTNAME(CpuTime)(CppTypeFor<TypeCategory::Real, 8> &time,
+    const char *sourceFile = nullptr, int line = 0);
----------------
klausler wrote:
> schweitz wrote:
> > rovka wrote:
> > > 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?
> > 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.
> 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. 
> 
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).


================
Comment at: flang/runtime/time-intrinsic.h:22
+void RTNAME(CpuTime)(CppTypeFor<TypeCategory::Real, 8> &time,
+    const char *sourceFile = nullptr, int line = 0);
+
----------------
klausler wrote:
> These source position arguments are needed only for runtime APIs that can encounter errors.  CPU_TIME probably can't.
Ok, removing them.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103805/new/

https://reviews.llvm.org/D103805