[SYCL][LIT] Mark spec const LIT as pass

[bjoernknafla]

The spec_const LIT tests start to pass for Intel CPU and GPU devices
with the Intel OpenCL GPU driver 20.19.16754.

Fixes #1873

Signed-off-by: Bjoern Knafla [email protected]

[bjoernknafla]

Exactly the two spec const LIT tests that start passing locally for OpenCL CPU and GPU are failing on the buildbots. Weird.

Both tests failled on the ACC.

[bjoernknafla]

Interesting that this passes for Windows but not for Linux.

[bader]

Another idea:

Suggested change

	// XFAIL: linux && opencl && !gpu && !cpu
	// XFAIL: linux && opencl && accelerator

[bjoernknafla]

This will make the test fail as expected though we do not get test coverage if it actually passes for CPU and GPU.

[bjoernknafla]

I wonder if to duplicate the test to make an accelerator-specific version with an XFAIL in it?

[bjoernknafla]

I guess the problem is that for OpenCL the test find cpu, gpu, and acceleator. Therefore the expression

// XFAIL: linux && opencl && !gpu && !cpu

evaluates as // XFAIL: false and (as both !gpu and !cpu are false) and therefore the test fails when the accelerator is not passing.

We could change the test to

// XFAIL: linux && opencl && accelerator

but then we do not see if it passes as we want for the CPU and GPU.

[v-klochkov]

@againull Do you have a good advise for this situation? 2 tests fail on linux+opencl+accelerator only.

[v-klochkov]

Artur (@againull) recommended trying '// XFAIL: linux && opencl && accelerator'.

[bader]

@v-klochkov, @againull, I think you are missing the point @bjoernknafla is trying to make.

// XFAIL: linux && opencl && accelerator can be interpreted as "expect fails if we run on Linux system with OpenCL ACCELERATOR device type". This is not very useful because if we have other device types available and test fails there, it won't be considered as a failure.

We need more flexible/configurable testing system, but I think it will require significant changes.
Meanwhile there are a few options how we can fix this test, but it will make testing approach inconsistent. Moving the check on accelerator device type is one option. Another option is just disable accelerator check in existing test source and remove XFAIL. Third, add new lit variable expecting to fail particular run e.g. %NOT_ACC_RUN_PLACEHOLDER == not %ACC_RUN_PLACEHOLDER.

[bjoernknafla]

What about this: we add extra test files named ..._accelerator.cpp. All these files do is:

• Add XFAIL: accelerator
• Include the assocaited cpp test file.

A bit unorthodox but prevents code duplication and makes it easy to move back to a single file later on. Gives us time until we rework the testing approach.

[v-klochkov]

What about this: we add extra test files named ..._accelerator.cpp. All these files do is:

• Add XFAIL: accelerator
• Include the assocaited cpp test file.

A bit unorthodox but prevents code duplication and makes it easy to move back to a single file later on. Gives us time until we rework the testing approach.

This approach looks the easiest and clear to me. I vote for it.
Alexey's third idea is interesting, but I not fully understand the implementation details and don't know how common the problem that is fixed in this PR is. If it happen to be met often, we can add a variable later.

[bjoernknafla]

Sorry that it took me so long to come back to this. I have put the discussed changes in place to:

• XFAIL for accelerators
• Catch regressions for OpenCL CPU and GPU devices.

[bjoernknafla]

Interesting, seems that clang-format testing can fail for LIT tests. Changing the formatting.

[bader]

Thanks!