Test patches from NVHPC by alisterj1138 · Pull Request #86 · OpenACCUserGroup/OpenACCV-V

alisterj1138 · 2025-03-04T16:41:53Z

Hello! Here are several updates and fixes for the tests. I did my best to keep the commits as independent and granular as possible, so if you want, you can hopefully cherry pick them without issue.

Most of the changes are fairly minor, here's a list of the ones that aren't:

factoring out devtest from many F90 tests -- this reduces code duplication; the very first commit in this PR is the base one for this change, some of the commits that come after depend on it, but I made a note in the commit messages (edit to add: I forgot that this also factors out the "is_possible" function, so the refactor of the atomic tests also depends on this commit)
a major refactor of the F90 atomic tests -- there was a lot of code duplication here, and all these tests followed the same pattern, so they've been rewritten with preprocessor macros (if you could go through each of these files, I'd appreciate it, there was a lot of copy-paste going on and I want to be sure everything got edited correctly)
major edits to declare_copyin/create.F90 -- the tests seemed to be expecting a separate module file, but I couldn't figure out how to make the testing harness do that, so these have been edited to contain the module, as well as other misc bug fixes

If you have questions about any of these edits, please let me know! Thanks again for this test suite!

…header file

… reduce code duplication

…xing some math

…t fine grained enough for a very strict test

…o a missing declaration

…t loop

…est for refactored devtest

…are.h and fixing associated tests

…le isn't needed, and some misc fixes

… is column-major; also updating devtest usage

This reverts commit ed23f08.

andrewkallai · 2025-04-28T21:24:17Z

@alisterj1138
The first step taken to review these tests has been to run each test (185 out of the 190 files) with NVHPC version 25.3.
The results of this test run were collected on a UDEL system and are posted at this link:
https://github.com/ajarmusch/OpenACC-Results-Viewer
Apparently 6 tests fail to pass at runtime, and 2 do not compile.
I would consider relooking into those 8 as to why they do not pass.
In the meantime, I have run the applicable tests with the gcc compiler, and I am cross checking any pass/fail counts with our own previous test run.
I am going through the tests individually as well to see which ones I can say look good from reviewing the spec and the runtime result.

alisterj1138 · 2025-04-30T15:00:01Z

Great, thank you!

I looked at the 8 failures and they're all ones we know about and are tracking -- they're expected failures with 25.3.

Please let me know if you have any questions!

andrewkallai · 2025-06-12T21:30:54Z

@alisterj1138
When compiling atomic_capture_expr_and_x_assign.F90 with gfortran (GNU Fortran (GCC) 15.1.0):

gfortran -fopenacc -cpp -lm -foffload='-lm' -o /global/u2/a/andrewka/my_fork/OpenACCV-V/build/atomic_capture_expr_and_x_assign.F90 /global/u2/a/andrewka/my_fork/OpenACCV-V/Tests/atomic_capture_expr_and_x_assign.F90

/global/u2/a/andrewka/my_fork/OpenACCV-V/Tests/common.Fh:63:29:
   63 |       if (.NOT. done(j) .AND. b(j) .eq. tmp) then
      |                             1
Error: Logicals at (1) must be compared with .eqv. instead of .eq.
/global/u2/a/andrewka/my_fork/OpenACCV-V/Tests/common.Fh:77:6:

   77 |   if (current .eq. final) then
      |      1
Error: Logicals at (1) must be compared with .eqv. instead of .eq.
/global/u2/a/andrewka/my_fork/OpenACCV-V/Tests/atomic_template.Fh:198:20:

  198 |         totals(i) = ATOMIC_OP(a(i, j), totals(i))
      |                    1
Error: !$OMP ATOMIC assignment operator must be binary +, *, -, /, .AND., .OR., .EQV. or .NEQV. at (1)

The compiler does not accept the logic comparison with .eq. and wants .eqv. to be used in common.Fh
It also does not want = to be used in atomic_template.Fh.

For Cray Fortran : Version 18.0.0:

ftn -h acc,noomp -h msgs -o /global/u2/a/andrewka/my_fork/OpenACCV-V/build/atomic_capture_expr_and_x_assign.F90/atomic_capture_expr_and_x_assign.F900 /global/u2/a/andrewka/my_fork/OpenACCV-V/Tests/atomic_capture_expr_and_x_assign.F90


ftn-303 ftn: ERROR VERIFY_ATOMIC_SEQUENCE, File = common.Fh, Line = 63, Column = 36 
  Data type LOGICAL is not allowed with LOGICAL for the operation "eq".


ftn-303 ftn: ERROR VERIFY_ATOMIC_SEQUENCE, File = common.Fh, Line = 77, Column = 15 
  Data type LOGICAL is not allowed with LOGICAL for the operation "eq".


ftn-807 ftn: ERROR TEST1, File = atomic_template.Fh, Line = 198, Column = 21 
  The statement following an OpenMP ATOMIC directive must contain the LHS scalar variable and an operator or expression on the RHS.


ftn-805 ftn: ERROR TEST1, File = atomic_template.Fh, Line = 201, Column = 19 
  The ATOMIC CAPTURE directive must be followed by a specified statement order consisting of update:capture, capture:update or capture:write.

The nvfortran compiler does not yield these errors.

I would recommend the changes which gfortran and ftn suggest should be applied to the .Fh files in order to ensure that correct fortran syntax is being used.
Consulting reference documentation such as here for Cray Fortran I think is most helpful for figuring out the reason why these are syntax errors.

alisterj1138 · 2025-07-07T17:22:53Z

Thanks for checking these fixes! I've fixed the headers to use .EQV./.NEQV. instead. I was able to test with gfortran and I'm fairly confident they should be good now. (There was also a typo/bug in one of the defines, so I fixed that too.)

There were also a couple tests that needed the same fix (many thanks to one of my coworkers for spotting them), so I included that fix as well.

Tests/acc_testsuite.h

Tests/acc_wait_any.c

Tests/parallel_create_zero.c

Tests/parallel_create_zero.cpp

Tests/acc_wait_any.cpp

Tests/serial_if.F90

andrewkallai · 2025-08-01T23:07:54Z

Tests/atomic_template.Fh

+#define MIN_X_EXPR_LIST 1004
+
+#if ATOMIC_OPTYPE == EXPR_PLUS_X
+#define ATOMIC_OP(a, b) (a + b)


Suggested change

#define ATOMIC_OP(a, b) (a + b)

#define ATOMIC_OP(a, b) a + b

and all other associated ADD expressions

andrewkallai · 2025-08-01T23:09:15Z

Tests/atomic_template.Fh

+#if ATOMIC_OPTYPE == EXPR_PLUS_X
+#define ATOMIC_OP(a, b) (a + b)
+#elif ATOMIC_OPTYPE == EXPR_MINUS_X
+#define ATOMIC_OP(a, b) (a - b)


Suggested change

#define ATOMIC_OP(a, b) (a - b)

#define ATOMIC_OP(a, b) a - b

and all other associated MINUS expressions

andrewkallai · 2025-08-01T23:09:39Z

Tests/atomic_template.Fh

+#elif ATOMIC_OPTYPE == EXPR_MINUS_X
+#define ATOMIC_OP(a, b) (a - b)
+#elif ATOMIC_OPTYPE == EXPR_TIMES_X
+#define ATOMIC_OP(a, b) (a * b)


Suggested change

#define ATOMIC_OP(a, b) (a * b)

#define ATOMIC_OP(a, b) a * b

and all other associated TIMES expressions

andrewkallai · 2025-08-01T23:10:07Z

Tests/atomic_template.Fh

+#elif ATOMIC_OPTYPE == EXPR_TIMES_X
+#define ATOMIC_OP(a, b) (a * b)
+#elif ATOMIC_OPTYPE == EXPR_DIVIDED_X
+#define ATOMIC_OP(a, b) (a / b)


Suggested change

#define ATOMIC_OP(a, b) (a / b)

#define ATOMIC_OP(a, b) a / b

and all other associated DIVIDE expressions

andrewkallai · 2025-08-01T23:12:19Z

@alisterj1138
I have reviewed the files, please look through the comments at your convenience.

alisterj1138 · 2025-08-06T18:33:35Z

I'm starting to go through all the comments today (thank you for going through everything so thoroughly!), I'll mark them resolved as I fix them and push the commit(s) once I'm done with everything.

alisterj1138 added 30 commits February 26, 2025 14:29

factoring out devtest from many F90 tests and putting it in a common …

45f46a8

…header file

major refactor of F90 atomic tests to all follow a common pattern and…

07b5cdf

… reduce code duplication

updating the precision on some C/C++ tests to be less strict, also fi…

b829b73

…xing some math

fixing acc_malloc.* to be less strict; acc_property_free_memory is no…

4abfc73

…t fine grained enough for a very strict test

fixing acc_copyin_with_len.F90 to use new devtest, plus a bit of math

182ddb6

fixing some set_device_type tests to use the interface correctly; als…

f9119a0

…o a missing declaration

serial_private.c/.cpp were missing a present clause

d22fd42

fixing typo in serial_private.F90

146afc7

adding missing variable initialization

2cd87b3

error count update line missing equals; err + 1 --> err += 1

0e3757b

fixing some math for serial/parallel_implicit_data_attributes

2a6caa1

changing routine gang to routine seq; cannot call gang routine withou…

b0cf8ea

…t loop

adding missing reduction variable initialization

e5fdda1

fixing some typos

2b90cc1

acc routine (fmin) is unnecessary

21f9c80

fixing a bug in the test checks for kernels_if.*; also updating F90 t…

4ad176a

…est for refactored devtest

adding missing allocations to C/C++ declare_function_scope_* tests

95fe15d

removing some duplicate declarations and macros in acc_testsuite_decl…

d9491cf

…are.h and fixing associated tests

major edits to declare_copyin/create.F90 so that a separate module fi…

68b6cd2

…le isn't needed, and some misc fixes

adding array slice syntax and swapping indices around because Fortran…

2cf67f4

… is column-major; also updating devtest usage

fixing loop indicies

15911e1

fixing a loop index

1445751

fixing one more set of loop indicies

a0c54f8

fixing a typo and adding a declare create for a global

bb3cd0a

adding vector_length clause to force single thread gangs

d125b88

missing pragma in acc_copyin_async.c

bd37ed4

shifting some pragmas around so acc_copyout_async* tests align

c0bf6cf

removing duplicate acc creates and adding a missing copyout

ed23f08

fixing array bounds in firstprivate for serial_firstprivate.c/.cpp

cde47ff

Revert "removing duplicate acc creates and adding a missing copyout"

d922263

This reverts commit ed23f08.

alisterj1138 added 2 commits March 3, 2025 10:23

adding missing copyouts and fixing a copyout missing the finalize

09c7058

fixing typo, = -> ==

7fd9b83

alisterj1138 marked this pull request as ready for review March 4, 2025 17:52

alisterj1138 added 4 commits March 6, 2025 13:51

correct fix for acc_wait_any this time

34ca287

fixing typo in parallel_create_zero

24bffa6

modifying acc_wait_any slightly to make more use of async

40d9cb1

fixing misc typos, including a loop index and a couple #ifndefs

7cb389b

Merge branch 'master' into testpatches; grabbing updates from main repo

3a513fe

Fixing logical atomic tests to use .EQV./.NEQV. instead of .EQ./.NE.

ab83024

ajarmusch mentioned this pull request Jul 23, 2025

Fix two issues in parallel_create_zero #107

Merged

erichkeane mentioned this pull request Jul 23, 2025

acc_testsuite.h and acc_testsuite_declare.h define the same things, and are included in multiple TUs; also missing 'scalar' declaration #105

Open

andrewkallai reviewed Aug 1, 2025

View reviewed changes

	#define ATOMIC_OP(a, b) (a + b)
	#define ATOMIC_OP(a, b) a + b

	#define ATOMIC_OP(a, b) (a - b)
	#define ATOMIC_OP(a, b) a - b

	#define ATOMIC_OP(a, b) (a * b)
	#define ATOMIC_OP(a, b) a * b

	#define ATOMIC_OP(a, b) (a / b)
	#define ATOMIC_OP(a, b) a / b

Conversation

alisterj1138 commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrewkallai commented Apr 28, 2025

Uh oh!

alisterj1138 commented Apr 30, 2025

Uh oh!

andrewkallai commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alisterj1138 commented Jul 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andrewkallai Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

andrewkallai Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

andrewkallai Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

andrewkallai Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

andrewkallai commented Aug 1, 2025

Uh oh!

alisterj1138 commented Aug 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

alisterj1138 commented Mar 4, 2025 •

edited

Loading

andrewkallai commented Jun 12, 2025 •

edited

Loading