openshmem-org / tests-sos

Sandia OpenSHMEM unit tests and performance testing suite
Other
6 stars 11 forks source link

Deprecated atomic tests check APIs that aren't in the spec #40

Open ronawho opened 4 months ago

ronawho commented 4 months ago

Tests like c11_test_shmem_atomic_fetch.c check APIs that aren't defined in the spec. e.g. https://github.com/openshmem-org/tests-sos/blob/2d4a704cda2f3fa042c634ff91f12ab2639f770c/test/unit/c11_test_shmem_atomic_fetch.c#L107-L115

calls shmem_fetch on types like size_t, but the spec says:

TYPE shmem_fetch(const TYPE *source, int pe);

where TYPE is one of {float, double, int, long, long long}

I'd be happy to make changes to the tests, but am not sure what the best approach is. The runtime selection of the fetch operation means even the non-deprecated tests end up with a "potential" call to the non-spec functions. Left to my own devices I might just split out the deprecated fetch into a new macro (example in details below), but wasn't sure if there's a more preferred approach.

```diff diff --git a/test/unit/c11_test_shmem_atomic_fetch.c b/test/unit/c11_test_shmem_atomic_fetch.c index 3c75256..d98cccb 100644 --- a/test/unit/c11_test_shmem_atomic_fetch.c +++ b/test/unit/c11_test_shmem_atomic_fetch.c @@ -40,12 +40,6 @@ enum op { FETCH = 0, ATOMIC_FETCH, CTX_ATOMIC_FETCH, ATOMIC_FETCH_NBI, CTX_ATOMIC_FETCH_NBI }; -#ifdef ENABLE_DEPRECATED_TESTS -#define DEPRECATED_FETCH shmem_fetch -#else -#define DEPRECATED_FETCH shmem_atomic_fetch -#endif - #define SHMEM_NBI_OPS_CASES(OP, TYPE) \ case ATOMIC_FETCH_NBI: \ shmem_atomic_fetch_nbi(&val, &remote, \ @@ -67,9 +61,6 @@ enum op { FETCH = 0, ATOMIC_FETCH, CTX_ATOMIC_FETCH, ATOMIC_FETCH_NBI, remote = (TYPE)mype; \ shmem_barrier_all(); \ switch (OP) { \ - case FETCH: \ - val = DEPRECATED_FETCH(&remote, (mype + 1) % npes); \ - break; \ case ATOMIC_FETCH: \ val = shmem_atomic_fetch(&remote, (mype + 1) % npes); \ break; \ @@ -88,6 +79,28 @@ enum op { FETCH = 0, ATOMIC_FETCH, CTX_ATOMIC_FETCH, ATOMIC_FETCH_NBI, } \ } while (0) +#define TEST_DEPRECATED_SHMEM_FETCH(OP, TYPE) \ + do { \ + static TYPE remote; \ + TYPE val; \ + const int mype = shmem_my_pe(); \ + const int npes = shmem_n_pes(); \ + remote = (TYPE)mype; \ + shmem_barrier_all(); \ + switch (OP) { \ + case FETCH: \ + val = shmem_fetch(&remote, (mype + 1) % npes); \ + break; \ + default: \ + printf("Invalid operation (%d)\n", OP); \ + shmem_global_exit(1); \ + } \ + if (val != (TYPE)((mype + 1) % npes)) { \ + printf("PE %i received incorrect value with " \ + "TEST_SHMEM_FETCH(%s, %s)\n", mype, #OP, #TYPE); \ + rc = EXIT_FAILURE; \ + } \ + } while (0) #else #define TEST_SHMEM_FETCH(OP, TYPE) @@ -99,20 +112,11 @@ int main(int argc, char* argv[]) { int rc = EXIT_SUCCESS; #ifdef ENABLE_DEPRECATED_TESTS - TEST_SHMEM_FETCH(FETCH, float); - TEST_SHMEM_FETCH(FETCH, double); - TEST_SHMEM_FETCH(FETCH, int); - TEST_SHMEM_FETCH(FETCH, long); - TEST_SHMEM_FETCH(FETCH, long long); - TEST_SHMEM_FETCH(FETCH, unsigned int); - TEST_SHMEM_FETCH(FETCH, unsigned long); - TEST_SHMEM_FETCH(FETCH, unsigned long long); - TEST_SHMEM_FETCH(FETCH, int32_t); - TEST_SHMEM_FETCH(FETCH, int64_t); - TEST_SHMEM_FETCH(FETCH, uint32_t); - TEST_SHMEM_FETCH(FETCH, uint64_t); - TEST_SHMEM_FETCH(FETCH, size_t); - TEST_SHMEM_FETCH(FETCH, ptrdiff_t); + TEST_DEPRECATED_SHMEM_FETCH(FETCH, float); + TEST_DEPRECATED_SHMEM_FETCH(FETCH, double); + TEST_DEPRECATED_SHMEM_FETCH(FETCH, int); + TEST_DEPRECATED_SHMEM_FETCH(FETCH, long); + TEST_DEPRECATED_SHMEM_FETCH(FETCH, long long); #endif /* ENABLE_DEPRECATED_TESTS */ TEST_SHMEM_FETCH(ATOMIC_FETCH, float); ```
ronawho commented 4 months ago

Alternative approach that doesn't duplicate as much:

diff --git a/test/unit/c11_test_shmem_atomic_fetch.c b/test/unit/c11_test_shmem_atomic_fetch.c
index 3c75256..d5b5669 100644
--- a/test/unit/c11_test_shmem_atomic_fetch.c
+++ b/test/unit/c11_test_shmem_atomic_fetch.c
@@ -40,11 +40,10 @@
 enum op { FETCH = 0, ATOMIC_FETCH, CTX_ATOMIC_FETCH, ATOMIC_FETCH_NBI,
           CTX_ATOMIC_FETCH_NBI };

-#ifdef ENABLE_DEPRECATED_TESTS
-#define DEPRECATED_FETCH shmem_fetch
-#else
-#define DEPRECATED_FETCH shmem_atomic_fetch
-#endif
+#define SHMEM_DEPRECATED_OPS_CASES(OP, TYPE)                    \
+      case FETCH:                                               \
+        val = shmem_fetch(&remote, (mype + 1) % npes);          \
+        break;

 #define SHMEM_NBI_OPS_CASES(OP, TYPE)                           \
       case ATOMIC_FETCH_NBI:                                    \
@@ -67,9 +66,7 @@ enum op { FETCH = 0, ATOMIC_FETCH, CTX_ATOMIC_FETCH, ATOMIC_FETCH_NBI,
     remote = (TYPE)mype;                                        \
     shmem_barrier_all();                                        \
     switch (OP) {                                               \
-      case FETCH:                                               \
-        val = DEPRECATED_FETCH(&remote, (mype + 1) % npes);     \
-        break;                                                  \
+      SHMEM_DEPRECATED_OPS_CASES(OP, TYPE)                      \
       case ATOMIC_FETCH:                                        \
         val = shmem_atomic_fetch(&remote, (mype + 1) % npes);   \
         break;                                                  \
@@ -104,16 +101,9 @@ int main(int argc, char* argv[]) {
   TEST_SHMEM_FETCH(FETCH, int);
   TEST_SHMEM_FETCH(FETCH, long);
   TEST_SHMEM_FETCH(FETCH, long long);
-  TEST_SHMEM_FETCH(FETCH, unsigned int);
-  TEST_SHMEM_FETCH(FETCH, unsigned long);
-  TEST_SHMEM_FETCH(FETCH, unsigned long long);
-  TEST_SHMEM_FETCH(FETCH, int32_t);
-  TEST_SHMEM_FETCH(FETCH, int64_t);
-  TEST_SHMEM_FETCH(FETCH, uint32_t);
-  TEST_SHMEM_FETCH(FETCH, uint64_t);
-  TEST_SHMEM_FETCH(FETCH, size_t);
-  TEST_SHMEM_FETCH(FETCH, ptrdiff_t);
 #endif /* ENABLE_DEPRECATED_TESTS */
+#undef SHMEM_DEPRECATED_OPS_CASES
+#define SHMEM_DEPRECATED_OPS_CASES(OP, TYPE)

   TEST_SHMEM_FETCH(ATOMIC_FETCH, float);
   TEST_SHMEM_FETCH(ATOMIC_FETCH, double);