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

[Emil Velikov]

On 3 June 2015 at 14:27, Francisco Jerez <currojerez at riseup.net> wrote:
> 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?
>
Humble reminder.

-Emil