[SYCL] Update parallel_for_range.cpp test

[smanna12]

The changes are due to the PR: intel/llvm#3836
where the attribute: reqd_work_group_size does not get propagated to the caller from device functions to match with SYCL 2020 spec.

Signed-off-by: Soumi Manna [email protected]

[vladimirlaz]

It looks like regression test. The test should fail until product change is fixed to effectively expose the issue.

[smanna12]

It looks like regression test. The test should fail until product change is fixed to effectively expose the issue.

Thanks @vladimirlaz for the reviews. I will add XFAIL:Linux in the test.

[vladimirlaz]

It looks like regression test. The test should fail until product change is fixed to effectively expose the issue.

Thanks @vladimirlaz for the reviews. I will add XFAIL:Linux in the test.

may be it is misunderstanding but my expectation is that the test change adds case which is fixed by compiler change.
So the test should fail until product change is submitted.

[vladimirlaz]

It looks like regression test. The test should fail until product change is fixed to effectively expose the issue.

Thanks @vladimirlaz for the reviews. I will add XFAIL:Linux in the test.

may be it is misunderstanding but my expectation is that the test change adds case which is fixed by compiler change.
So the test should fail until product change is submitted.

@smanna12 could you please update the test to let it fail before intel/llvm#3836 is submitted and pass once the PR submitted?

[vladimirlaz]

@smanna12 could you please rebase the PR and apply comment?

[smanna12]

@smanna12 could you please rebase the PR and apply comment?

Sorry @vladimirlaz for replying late. Yes, i will rebase and apply comment. Thanks

[smanna12]

@vladimirlaz , i would like to follow up with you before i make the test change here:

In SYCL 1.2.1 spec (SYCL mode is 2017), the attributes get propagated from device functions to a kernel.

Example 1:  OK
[[cl::reqd_work_group_size(4, 4, 4)]] void reqd_wg_size_helper() {
  // do nothing
}

try {
        Q.submit([&](handler &CGH) {
          CGH.parallel_for<class ReqdWGSizeNegativeA>(
              nd_range<3>(range<3>(16, 16, 16), range<3>(8, 8, 8)),
              [=](nd_item<3>) { reqd_wg_size_helper(); });
        });

The SYCL 2020 ((SYCL mode is 2020), requirement mandating the avoidance of the propagation of all kernel attributes to the caller when used on a function.

Example 1: Fail because [[cl::reqd_work_group_size(4, 4, 4)]] Does not get propagated to the kernel.
[[cl::reqd_work_group_size(4, 4, 4)]] void reqd_wg_size_helper() {
  // do nothing
}

try {
        Q.submit([&](handler &CGH) {
          CGH.parallel_for<class ReqdWGSizeNegativeA>(
              nd_range<3>(range<3>(16, 16, 16), range<3>(8, 8, 8)),
              [=](nd_item<3>) { reqd_wg_size_helper(); });
        });

The default version in SYCL mode is 2020, so “llvm-test-suite/SYCL/Basic/ parallel_for_range.cpp” is failing due to the PR: intel/llvm#3836

Proposed fix:

1. Pass -sycl-std=2017 to the RUN line so that propagating the attribute happens to the kernel from device function.
2. Update test (similar like reqd_work_group_size.cpp) to pass the attribute directly to the kernel instead of calling through device function so that we do not need to pass any sycl-std= to the command line.

try {
        Q.submit([&](handler &CGH) {
          CGH.parallel_for<class ReqdWGSizeNegativeA>(
              nd_range<3>(range<3>(16, 16, 16), range<3>(8, 8, 8)),
              [=](nd_item<3>) { [[sycl::reqd_work_group_size(4, 4, 4)]]});
        });

3. Add // XFAIL:

I do not think we can add any test case which is fixed by compiler change, So the test should fail until product change is submitted.

Please let me know what do you think about the correct fix here? Thanks

[vladimirlaz]

Please let me know what do you think about the correct fix here? Thanks

We are moving to SYCL2020 so I suggest to have only 2020-compatible test

[smanna12]

Please let me know what do you think about the correct fix here? Thanks

We are moving to SYCL2020 so I suggest to have only 2020-compatible test

Thanks @vladimirlaz for the suggestion. I have created #353 for this. Thanks.