[Piglit] [PATCH 02/10] cl: Add sign_extend_inreg test

[Jan Vesely]

On Tue, 2016-12-06 at 13:33 -0800, Matt Arsenault wrote:
> > On Dec 6, 2016, at 12:18, Jan Vesely <jan.vesely at rutgers.edu> wrote:
> > 
> > On Tue, 2016-12-06 at 10:55 -0800, Matt Arsenault wrote:
> > > > On Dec 5, 2016, at 13:20, Jan Vesely <jan.vesely at rutgers.edu <mailto:jan.vesely at rutgers.edu>> wrote:
> > > > 
> > > > On Mon, 2016-12-05 at 09:48 -0800, arsenm2 at gmail.com <mailto:arsenm2 at gmail.com> <mailto:arsenm2 at gmail.com <mailto:arsenm2 at gmail.com>> wrote:
> > > > > From: Matt Arsenault <arsenm2 at gmail.com <mailto:arsenm2 at gmail.com> <mailto:arsenm2 at gmail.com <mailto:arsenm2 at gmail.com>>>
> > > > > 
> > > > > ---
> > > > > tests/cl/program/execute/sign_extend_inreg.cl | 387 ++++++++++++++++++++++++++
> > > > > 1 file changed, 387 insertions(+)
> > > > > create mode 100644 tests/cl/program/execute/sign_extend_inreg.cl
> > > > 
> > > > this looks very GCN specific, the name should IMO indicate it.
> > > 
> > > It’s completely a completely generic test, just the testcases are
> > > intended to stress the important cases for GCN
> > 
> > there is no sign extent CL operation, nor sign extend inreg. CL
> > implementations are not required to have SGPR registers. Almost all of
> > the tests in this series are GCN specific especially with the names
> > like v_* and s_*.
> > 
> > I'm not against GCN specific test cases, I'm just saying that it should
> > be marked as such. Just like "r600 create release buffer bug", the test
> > is generic (runs and passes on other platforms), but tests r600
> > specific bug/behaviour. It would also make it easier to use regexp to
> > select/skip only these specific tests.
> > 
> > Jan
> 
> It’s testing the underlying operation in the compiler exposed by
> using the basic bit operations, the fact that there isn’t an explicit
> CL operation called sign extend is irrelevant. Skipping these on
> other platforms would be a mistake. The s_/v_ distinction is to
> emphasize that it is stressing the scalar operators which the
> standard conformance tests will not do.There are more tests because
> of the emphasize on stressing the GCN compiler parts, but nothing
> about it is specific.

yes, compiler parts specific to GCN. I don't see how the distinction
between s_ and v_ would be useful for CPU or FPGA implementation of
OpenCL. I agree that the tests should run and pass on all platforms
(the r600 buffer bug test also runs and passes on other platforms),
they are just less interesting on non-GCN hw, so they should be marked
with a distinct name.

why should beignet, or r600, be interested in a test that targets 64-
bit s_loads, or puts parameters in SGRPs?

Naming is a minor issue, but I think it would help clarify the purpose 
of these tests. It's just a suggestion though.


Jan


> 
> -Matt
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/piglit/attachments/20161206/bc8fba2c/attachment-0001.sig>