[Mesa-dev] [PATCH 1/2] i965: Remove hand-rolled memcpy implementation.

[Ian Romanick]

On 02/23/2015 12:14 AM, Kenneth Graunke wrote:
> On Sunday, February 22, 2015 08:40:17 PM Matt Turner wrote:
>> ---
>> I can't believe this is beneficial, especially since it came from the
>> i915tex import.
>>
>>  src/mesa/drivers/dri/i965/brw_context.h       | 27 ---------------------------
>>  src/mesa/drivers/dri/i965/intel_batchbuffer.c |  2 +-
>>  2 files changed, 1 insertion(+), 28 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>> index 197ed3e..7e86c1b 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -1920,33 +1920,6 @@ gen6_upload_push_constants(struct brw_context *brw,
>>                             struct brw_stage_state *stage_state,
>>                             enum aub_state_struct_type type);
>>  
>> -/* ================================================================
>> - * From linux kernel i386 header files, copes with odd sizes better
>> - * than COPY_DWORDS would:
>> - * XXX Put this in src/mesa/main/imports.h ???
>> - */
>> -#if defined(i386) || defined(__i386__)
>> -static inline void * __memcpy(void * to, const void * from, size_t n)
>> -{
>> -   int d0, d1, d2;
>> -   __asm__ __volatile__(
>> -      "rep ; movsl\n\t"
>> -      "testb $2,%b4\n\t"
>> -      "je 1f\n\t"
>> -      "movsw\n"
>> -      "1:\ttestb $1,%b4\n\t"
>> -      "je 2f\n\t"
>> -      "movsb\n"
>> -      "2:"
>> -      : "=&c" (d0), "=&D" (d1), "=&S" (d2)
>> -      :"0" (n/4), "q" (n),"1" ((long) to),"2" ((long) from)
>> -      : "memory");
>> -   return (to);
>> -}
>> -#else
>> -#define __memcpy(a,b,c) memcpy(a,b,c)
>> -#endif
>> -
>>  #ifdef __cplusplus
>>  }
>>  #endif
>> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> index 7b96005..45c7493 100644
>> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> @@ -390,7 +390,7 @@ intel_batchbuffer_data(struct brw_context *brw,
>>  {
>>     assert((bytes & 3) == 0);
>>     intel_batchbuffer_require_space(brw, bytes, ring);
>> -   __memcpy(brw->batch.map + brw->batch.used, data, bytes);
>> +   memcpy(brw->batch.map + brw->batch.used, data, bytes);
>>     brw->batch.used += bytes >> 2;
>>  }
>>  
>>
> 
> I submitted an identical patch in September 2013:
> http://lists.freedesktop.org/archives/mesa-dev/2013-September/045360.html
> 
> Ian seemed to think it was for atom systems, and suggested it might be
> useful to use on 64-bit atoms as well.  He also requested benchmarking
> proving it wasn't useful rather than my handwaving:
> 
> http://lists.freedesktop.org/archives/mesa-dev/2013-September/045516.html
> 
> However, in the i965 driver, it appears to only be used for:
> - glBitmap BLT commands
> - Gen4 URB fences
> So, unless I'm wrong, this is so irrelevant that I don't have any qualms
> about nuking it.
> 
> In i915c, it's apparently used for a lot more.  I really can't get
> motivated about benchmarking CPU overhead on 8xx/9xx, though...

I guess we could land it and see of the automated test systems pick up
any change...

> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150223/a2932710/attachment.sig>