Run test_omp_target_offload with multiple environment vars set

[tob2]

Currently, 5.0/program_control/test_omp_target_offload.c is a bit pointless as it requires that OMP_TARGET_OFFLOAD is set, which is usually not the case.

As discussed in pull #240, it cannot be set after program start.

This patch runs the once-compiled program three times. It seems to work okay, but I am not sure whether it messes up any diagnostic postprocessing.

• sys/scripts/run_test.sh: Permit passing an environment variable.
• Makefile (test_omp_target_offload.c.run, test_omp_target_offload.c.runonly):
  Run trice with env OMP_TARGET_OFFLOAD set to DEFAULT, MANDATORY, DISABLED.

[tob2]

@spophale , @jhdavis8 , and @tmh97: Does this make sense – or could it be done more sensible?

I think one item which probably should be improved is the dependency of "run" – it simply requires now all input files instead of only the one required. There are probably more issues.

[jhdavis8]

@tob2 This looks good to me. I want to double-check that it will interact OK with the results collection, there might be some overwrite/naming issues with the output files. Also, is there a way to do this with a bash FOR, to reduce the repetition?

For the run dependencies: If I understand your suggestion correctly, I believe the SOURCES option, if specified, will change the run rule dependency list to be just the listed files.

[tob2]

I have now moved it to the Bash script using --iter-env; I think it looks cleaner. As it simply runs the script multiple times before printing the PASS/FAIL line, it should work with all reporting scripts.

NOTE: I forced-pushed the new version. The previous version is at SOLLVE/sollve_vv@master...tob2:target_offload_env-orig

The make output now looks as follows. ("PASS" is placed between the other logs as STDERR is unbuffered contrary to STDOUT.)

running: bin/test_omp_target_offload.c.run 
sys/scripts/run_test.sh --iter-env OMP_TARGET_OFFLOAD MANDATORY,DISABLE,DEFAULT bin/test_omp_target_offload.c.o 1 | tee -a logs/test_omp_target_offload.c.log && echo "PASS" > _ompvv_temp_result_.exitstatus.5173 || echo "FAIL" > _ompvv_temp_result_.exitstatus.5173
test_omp_target_offload.c.o:

[OMPVV_INFO: test_omp_target_offload.c:54] Test is running on host.
[OMPVV_INFO: test_omp_target_offload.c:94] OMP_TARGET_OFFLOAD has been set to MANDATORY
test_omp_target_offload.c.o: PASS. exit code: 0
[OMPVV_INFO: test_omp_target_offload.c:96] The value of errors is 0.
[OMPVV_RESULT: test_omp_target_offload.c] Test passed on the host.

[OMPVV_INFO: test_omp_target_offload.c:54] Test is running on host.
[OMPVV_WARNING: test_omp_target_offload.c:92] OMP_TARGET_OFFLOAD has been set to DISABLED
[OMPVV_INFO: test_omp_target_offload.c:96] The value of errors is 0.
[OMPVV_RESULT: test_omp_target_offload.c] Test passed on the host.

[OMPVV_INFO: test_omp_target_offload.c:54] Test is running on host.
[OMPVV_INFO: test_omp_target_offload.c:94] OMP_TARGET_OFFLOAD has been set to DEFAULT
[OMPVV_INFO: test_omp_target_offload.c:96] The value of errors is 0.
[OMPVV_RESULT: test_omp_target_offload.c] Test passed on the host.

[Re-committed + fixed ↑ as I had a typo in of the "DISABLED" (missing tailing 'D' in one place).]

[tob2]

One question: Currently, it only checks DEFAULT, MANDATORY, DISABLED; should it also check 'unset'?

[tob2]

Early ping

[spophale]

In the test we do capture if the value is not set. The 5.0 spec only recognizes DEFAULT, MANDATORY, DISABLED and the language suggests that OMP_TARGET_OFFLOAD must be one of those.

TODO: We add an error if it is not set.

Thanks @tob2 !

[jhdavis8]

I tested this out. Looks like the way our results generation works causes all the results from the three runs to be merged into one result, with the final pass/fail decision based on the outcome of the last run (I need to check this further to see exactly what is going on, it could be that it needs all three to pass in order to pass).

We need to discuss what the expected behavior should be in terms of the website results. Should the website show three rows per compiler/system for this test? If so, right now there is no way to distinguish the environment variable settings on the website. Or, should it just show PASS/FAIL based on whether or not all three runs pass? The latter is my preference.

TL;DR this does basically work with our full infrastructure, but we should discuss what exactly we want to do on the website with this. @spophale @tmh97

[tob2]

We need to discuss what the expected behavior should be in terms of the website results. [... ]TL;DR this does basically work with our full infrastructure, but we should discuss what exactly we want to do on the website with this. @spophale @tmh97

@jhdavis8 – any news?

[jhdavis8]

Yes, we did discuss it last meeting, I forgot to mention here what we decided. We're thinking it would work best to create three separate test files, one for each setting, each with one unique run rule in the Makefile. That way the online results are clear about which environment settings fail/pass, since the test names will indicate which setting was used.

[tob2]

We're thinking it would work best to create three separate test files, one for each setting, each with one unique run rule in the Makefile.

Like this new version?

[spophale]

LGTM.
@jhdavis8 will you be able to test this ?

[tob2]

• Last commit: Fixed two small issues: passing an enum to %s did not make sense + removed a compiler warning (unused var).
• I wonder whether a OMPVV_TEST_AND_SET(errors, policy!=EXPECTED_POLICY); should be added in addition.
• BTW: I tested it with GCC mainline (GCC 11), albeit on a system without an offloading device.

[jhdavis8]

@spophale just tested on Summit. Works as expected.

[tmh97]

Hey @tob2 , I realized some of the changes we pushed into the makefile broke the ability to run .c files using make ... compile followed bymake ... run

Specifically the lines 350-368 as noted in this issue #400

It seems the makefile error is thrown on line 358.

Any thoughts on what may be the issue? I'll continue to look into this but figured it was worth asking