[Piglit] [PATCH] Add OpenCL support

Eric Anholt eric at anholt.net
Wed Sep 19 01:48:56 PDT 2012


Tom Stellard <tom at stellard.net> writes:

> On Wed, Sep 12, 2012 at 09:01:43AM -0700, Kenneth Graunke wrote:
>> On 09/12/2012 07:51 AM, Tom Stellard wrote:
>> > On Thu, Sep 06, 2012 at 10:44:18PM +0200, Blaž Tomažič wrote:
>> >> On čet, 2012-09-06 at 09:16 -0700, Eric Anholt wrote:
>> >>> Blaž Tomažič <blaz.tomazic at gmail.com> writes:
>> >>>
>> >>>> On tor, 2012-09-04 at 07:14 -0700, Tom Stellard wrote:
>> >>>>> Some of the cl-util files contain some really long lines, especially
>> >>>>> piglit-framework-cl-program.c:96-99.  You should go through and wrap some
>> >>>>> of the longer lines at 80 characters to make them easier to read.
>> >>>>
>> >>>> I went through all the code and wrapped as much of it as possible to 80
>> >>>> lines.
>> >>>>
>> >>>> The new fixed branch is now at:
>> >>>>         git://github.com/blazt/piglit.git opencl-request-v2
>> >>>
>> >>> I haven't read through everything, but I've looked at the docs and
>> >>> browsed some commits and I like it.  I'd be happy to see it merged.
>> >>>
>> >>> A few recommendations I'd give for further developent:
>> >>>
>> >>> Add a bit of python too all_cl.tests to autodetect your .cl tests.  It's
>> >>> irritating when writing tests to remember to add it, and you end up
>> >>> mistyping a filename and not running all_cl.tests and pushing the code
>> >>> with a broken all_cl.tests. (This means losing nicely-formatted names
>> >>> in the results, and replacing that with filenames.
>> >>
>> >> I can add an option to query for test's name since you can define it
>> >> in .cl files. This way we don't lose nicely-formatted names. Or even
>> >> better, use the option you provided next.
>> >>
>> >>> I've been thinking
>> >>> it would be good to add support to the framework for part of the test
>> >>> result to be the name to report the result under, which would open the
>> >>> way for a single test binary that tests a few small things, or
>> >>> variations, and reports them separately)
>> >>
>> >> Do you mean just adding something like this to the output of a test:
>> >>         PIGLIT: { 'name': 'Name', 'result': 'pass' }
>> >> so the test binary can name itself (depending on each different run)?
>> >>
>> >> Or also that a test binary can report that line multiple times with
>> >> different names? For example to report results of "subtests".
>> >>
>> >>> Drop the "plain" thing in all_cl.tests.  It's an uninteresting artifact
>> >>> of the class name that happens to do the work.
>> >>>
>> >>> Always use concurrent testing when possible.  As you get more and more
>> >>> tests, being able to light up all your cores for testing is really nice.
>> >>
>> >> This is meant for further development or to fix the current pull
>> >> request?
>> >>
>> > 
>> > Hi Eric,
>> > 
>> > Is the code ready to be pulled as is or does Blažneed to make these
>> > changes first?
>> > 
>> > -Tom
>> 
>> Eric's on vacation and then immediately heading to XDC, so I doubt he'll
>> be able to respond any time soon.
>> 
>> In my mind, these all seem like things that could get fixed in follow-on
>> patches.
>
> In that case, would you, or someone else with commit access be able to
> merge this branch?

I've pushed it.  Thanks to Blaž for the great work!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20120919/b74fa65c/attachment.pgp>


More information about the Piglit mailing list