OpenMP-Validation-and-Verification / OpenMP_VV

OpenMP Offloading Validation & Verification Suite; Official repository. We have migrated from bitbucket!! For documentation, results, publication and presentations, please check out our website ->
https://crpl.cis.udel.edu/ompvvsollve/
Other
54 stars 19 forks source link

Adding test case for target teams distribute parallel for default con… #783

Closed lthakur007 closed 5 months ago

lthakur007 commented 11 months ago

Please review...

spophale commented 11 months ago

@lthakur007 , this appears to be a OpenMP 5.1 test as before that only shared or none was allowed with default clause for C/C++. According to 5.1 the default clause applies to both teams and parallel. The test logic needs to be checked for such usage.

lthakur007 commented 10 months ago

I have moved the test to 5.1 folder. Will observe CI machines report with 5.1 version of this test...

lthakur007 commented 10 months ago

HI @spophale , I have addressed the review comments, please consider the changes...

seyonglee commented 10 months ago

Please delete the obsolete test: tests/4.5/target_teams_distribute_parallel_for/test_target_teams_distribute_parallel_for_default.c

lthakur007 commented 9 months ago

Please delete the obsolete test: tests/4.5/target_teams_distribute_parallel_for/test_target_teams_distribute_parallel_for_default.c

Done!! Thank you @seyonglee :) ..

lthakur007 commented 9 months ago

Hi @spophale , Addressed all the review comments.. Please consider the updates ...

lthakur007 commented 9 months ago

Hi @spophale , Please consider the changes submitted....

fel-cab commented 8 months ago

It is not clear to me how this test is testing the default clause, since the test would pass if the default clauses are removed, or they are changed for the other possible default (e.g. if the default(private) is changed for default(shared) or the other way around). Is it possible to change the test in a way that the consequence of default is carefully tested?

lthakur007 commented 8 months ago

It is not clear to me how this test is testing the default clause, since the test would pass if the default clauses are removed, or they are changed for the other possible default (e.g. if the default(private) is changed for default(shared) or the other way around). Is it possible to change the test in a way that the consequence of default is carefully tested?

Hi @spophale , I agree with the comment of @fel-cab mentioned above. However, the test passes with/without "default(firstprivate)" and also with/without presence of "map(to: Arr[0:32])". I have attached a file for reference. The terminal output when the file is run is as follows: @spophale , could you please help me improve the test case.....

_/opt/rocm/llvm/bin/clang -O2 -fopenmp --offload-arch=gfx906 -fopenmp-version=51 dist_paraleldefault.c ./a.out Test Passed!! Test Passed!! Test Passed!! Test Passed!! __ dist_paralel_default.txt

seyonglee commented 8 months ago

I also agree that this test does not actually test the default clause, since the default clause is not applied to the target construct (and this test checks the data mapping behaviors only.)

spophale commented 8 months ago

@lthakur007 , here is my suggestion: For default(firstprivate)

Same steps can be used for default(private) but CONST will be assigned inside the target region -- 'CONST = team_num + thread_num'

spophale commented 5 months ago

This test was merged with PR #793 .This is a duplicate.