Page MenuHomePhabricator

[OpenCL] Launch opencl-types.cl test only on x86
Needs ReviewPublic

Authored by AlexeySachkov on Nov 8 2018, 2:53 AM.

Details

Summary

There is no sence to launch this test on targets that aren't supposed to
support it.

Diff Detail

Event Timeline

AlexeySachkov created this revision.Nov 8 2018, 2:53 AM

Are these types represented differently wrt targets?

If I understand correctly, not all extensions are available on non-x86 targets and some declarations are marked as (invalid) - that is the only difference I saw

I am a bit confused about this testing since you are running OpenCL on architectures that aren't supposed to support it. Can you provide more details on why you are doing this?

I'm running OpenCL on an x886 and everything is fine, but there are a lot of build bots which build different targets on different architectures. Since there are no target option specified, c-index-test uses native target.

I just realized: may be it is better to add something like // REQUIRES: x86 to the test?

I'm running OpenCL on an x886 and everything is fine, but there are a lot of build bots which build different targets on different architectures. Since there are no target option specified, c-index-test uses native target.

I just realized: may be it is better to add something like // REQUIRES: x86 to the test?

Yes, it's the same for clang actually. We are solving this by adding -triple explicitly, alternatively REQUIRES will work too.

AlexeySachkov retitled this revision from [OpenCL][NFC] Improve test coverage of test/Index/opencl-types.cl to [OpenCL] Launch opencl-types.cl test only on x86.
AlexeySachkov edited the summary of this revision. (Show Details)

Simplified patch. Updated title and description

FWIW, I'd vote for the first revision of this patch. From my
understanding, the test verifies that libclang is able to parse OpenCL
code correctly. It doesn't do anything specific to x86: target for x86 just
happens to support a set of OpenCL extensions.

FWIW, I'd vote for the first revision of this patch. From my
understanding, the test verifies that libclang is able to parse OpenCL
code correctly. It doesn't do anything specific to x86: target for x86 just
happens to support a set of OpenCL extensions.

I am trying to understand what exactly does it bring into testing if the code doesn't have anything target specific in there?

Tests are not entirely free, so we can't test absolutely everything.

FWIW, I'd vote for the first revision of this patch. From my

understanding, the test verifies that libclang is able to parse OpenCL
code correctly. It doesn't do anything specific to x86: target for x86 just
happens to support a set of OpenCL extensions.

I am trying to understand what exactly does it bring into testing if the code doesn't have anything target specific in there?

It brings the architecture that does not support any OpenCL extension,
thus we check that types are "invalid".

Tests are not entirely free, so we can't test absolutely everything.

Right. There is no big difference b/w these two revisions, so I'll leave
that to your discretion.

FWIW, I'd vote for the first revision of this patch. From my

understanding, the test verifies that libclang is able to parse OpenCL
code correctly. It doesn't do anything specific to x86: target for x86 just
happens to support a set of OpenCL extensions.

I am trying to understand what exactly does it bring into testing if the code doesn't have anything target specific in there?

It brings the architecture that does not support any OpenCL extension,
thus we check that types are "invalid".

Ok, let's test one invalid configuration if you think it's valuable. I assume we can't test all?