[Mesa-dev] [Mesa-stable] [PATCH 2/6] clover: Call clBuildProgram() notification function when build completes v2

[Francisco Jerez]

Emil Velikov <emil.l.velikov at gmail.com> writes:

> Hi Tom,
>
> On 31 March 2015 at 15:29, Francisco Jerez <currojerez at riseup.net> wrote:
>> Tom Stellard <thomas.stellard at amd.com> writes:
>>
>>> v2:
>>>   - Only call notification for build errors
>>>   - Fix clCompileProgram()
>>>
>>> Cc: 10.5 10.4 <mesa-stable at lists.freedesktop.org>
>>> ---
>>>  src/gallium/state_trackers/clover/api/program.cpp | 16 ++++++++++++++--
>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/gallium/state_trackers/clover/api/program.cpp b/src/gallium/state_trackers/clover/api/program.cpp
>>> index 60184ed..5cd543c 100644
>>> --- a/src/gallium/state_trackers/clover/api/program.cpp
>>> +++ b/src/gallium/state_trackers/clover/api/program.cpp
>>> @@ -180,12 +180,18 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
>>>     validate_build_program_common(prog, num_devs, d_devs, pfn_notify, user_data);
>>>
>>>     prog.build(devs, opts);
>>> +   if (pfn_notify)
>>> +      pfn_notify(d_prog, user_data);
>>
>> Maybe leave blank lines around the conditional where preceded/followed
>> by another statement?
>>
>>>     return CL_SUCCESS;
>>> +} catch (const build_error &e) {
>>> +   if (pfn_notify)
>>> +      pfn_notify(d_prog, user_data);
>>> +   if (e.get() == CL_COMPILE_PROGRAM_FAILURE)
>>> +      return CL_BUILD_PROGRAM_FAILURE;
>>> +   return e.get();
>>
>> You can just return CL_BUILD_PROGRAM_FAILURE here unconditionally.
>>
>>>  } catch (error &e) {
>>>     if (e.get() == CL_INVALID_COMPILER_OPTIONS)
>>>        return CL_INVALID_BUILD_OPTIONS;
>>> -   if (e.get() == CL_COMPILE_PROGRAM_FAILURE)
>>> -      return CL_BUILD_PROGRAM_FAILURE;
>>>     return e.get();
>>>  }
>>>
>>> @@ -223,8 +229,14 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
>>>        objs<allow_empty_tag>(d_header_progs, num_headers));
>>>
>>>     prog.build(devs, opts, headers);
>>> +   if (pfn_notify)
>>> +      pfn_notify(d_prog, user_data);
>>
>> Same here.
>>
> Did you had the chance to address Francisco's comments ? Did this
> patch fell through the cracks, or should I consider it
> obsolete/rejected ?
>

I think the patch was good it just fell through the cracks.  Tom?

> Thanks
> Emil
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150603/4dae731b/attachment.sig>