Re: [PATCH] drm/i915: prevent out of range pt in the PDE macros (take 3)

[Michel Thierry <michel.thierry@intel.com>]

On 10/2/2015 1:58 PM, Chris Wilson wrote:
> On Fri, Oct 02, 2015 at 01:47:03PM +0100, Michel Thierry wrote:
>> On 10/1/2015 5:09 PM, Chris Wilson wrote:
>>> On Thu, Oct 01, 2015 at 04:59:35PM +0100, Michel Thierry wrote:
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_gem_gtt.h | 14 ++++++++++----
>>>>   1 file changed, 10 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
>>>> index 9fbb07d..94f8344 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
>>>> @@ -394,7 +394,9 @@ struct i915_hw_ppgtt {
>>>>    */
>>>>   #define gen6_for_each_pde(pt, pd, start, length, temp, iter) \
>>>>   	for (iter = gen6_pde_index(start); \
>>>> -	     pt = (pd)->page_table[iter], length > 0 && iter < I915_PDES; \
>>>> +	     pt = (length > 0 && iter < I915_PDES) ? \
>>>> +			(pd)->page_table[iter] : NULL, \
>>>> +	     length > 0 && iter < I915_PDES; \
>>>
>>> length > 0 && iter < I915_PDES ? (pt = (pd)->page_table[iter]) : 0,
>>>
>>> as the compiler wouldn't be able to CSE it otherwise (I think).
>>
>> Even after that change, the compiler keeps doing an optimization
>> when page_table[iter] is null (takes the null assignment as the
>> break condition).
>>
>> I've been playing with these examples
>> http://paste.ubuntu.com/12638106/
>>
>> Only the 1st example (a) iterates over all elements, b & c stop
>> after the 1st run.
>
> Forgot that was the condition you wanted to change.
>
> length > 0 && iter < I915_PDES ? (pt = (pd)->page_table[iter]), 1 : 0,
>
> Would be nice to prove that length > 0 implies iter < I915_PDES. If only
> we had smart tools :)

Thanks, that made it.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx