On 10/16/2013 04:03 PM, Ian Romanick wrote:
On 10/16/2013 03:45 PM, Chad Versace wrote:
On 10/15/2013 04:33 PM, Jordan Justen wrote:
Signed-off-by: Jordan Justen <jordan.l.jus...@intel.com>
Cc: Chad Versace <chad.vers...@linux.intel.com>
---
Chad, this conflicts with your "Detect if system has POSIX clocks" patch,
but it could be tweaked to follow your version.
I'm not opposed to defining a piglit_get_microseconds(). However, I'd
rather layer it on top of my already submitted POSIX clocks patch.
Why is one approach for detecting clock_gettime better or worse than the
other? I assume it's more than just whatever you saw when you opened
the coffin lid on check_function_exists...
Issues below...
CMakeLists.txt | 6 ++++++
tests/util/piglit-util.c | 17 +++++++++++++++++
tests/util/piglit-util.h | 8 ++++++++
3 files changed, 31 insertions(+)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4ec5ddf..c876a05 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -6,6 +6,7 @@ INCLUDE (CheckCCompilerFlag)
INCLUDE (CheckCXXCompilerFlag)
INCLUDE (CheckFunctionExists)
INCLUDE (CheckIncludeFile)
+INCLUDE (CheckLibraryExists)
INCLUDE (FindPkgConfig)
project (piglit)
@@ -294,6 +295,11 @@ check_function_exists(strchrnul HAVE_STRCHRNUL)
check_function_exists(strndup HAVE_STRNDUP)
check_function_exists(fopen_s HAVE_FOPEN_S)
check_function_exists(setrlimit HAVE_SETRLIMIT)
+check_function_exists(clock_gettime HAVE_CLOCK_GETTIME_FUNC)
I just inspected CMake's implementation of check_function_exists(),
and now I am strongly against using it anywhere. If you saw its
implementation,
you would likely agree too. You find it in
/usr/share/cmake/Modules/CheckFunctionExists.{c,cmake}.
Well... they try to support pre-ANSI C, so that's a strike against them.
It's fairly ugly, but, off the top of my head, I can't think of a
better way that is still generic.
Do you see anything wrong with this? Not only does it check that the
symbol is present, but it checks that all the pieces are there including
the headers and symbols we want.
check_c_source_compiles(
HAVE_CLOCK_GETTIME
"
#define POSIX_C_SOURCE >= 199309L
#include <time.h>
int main() { return clock_gettime(CLOCK_MONOTONIC, NULL); }
"
)
If a library is required for linking, then you can pass that in too.
+uint64_t
+piglit_get_microseconds(void)
+{
+#ifdef HAVE_CLOCK_GETTIME
+ struct timespec t;
+ clock_gettime(CLOCK_MONOTONIC, &t);
+ return (t.tv_sec * 1000000) + (t.tv_nsec / 1000);
+#else
+ return (uint64_t) 0;
+#endif
+}
I'm opposed to implementing broken functions. If this function is not yet
implemented for some system, then tests that require it should either skip
or not even build. This patch's approach, though, causes such tests to
fail.
I think having piglit_get_microseconds return int64_t, where -1 is
error, will fix the problem. Then tests can do:
if ((t = piglit_get_microseconds()) < 0) {
// skip
}
This also lets us handle the case where clock_gettime returns an error.
That's a sensible approach.
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/piglit
