<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN"> <html> <head> <meta content="text/html; charset=ISO-8859-1" http-equiv="Content-Type"> </head> <body text="#000000" bgcolor="#ffffff"> Hi, thanks for reviewing these patches! I wrote some comments below. On 09/27/2011 11:01 PM, Søren Sandmann wrote: <blockquote cite="mid:ye8sjnit784.fsf@llama04.cs.au.dk" type="cite"> <pre wrap="">The one highlevel comment I have is about fast_composite_solid_fill(), where you change from calling pixman_fill() to directly calling the new pixman_fill_fast_path(). This is problematic because pixman_fill() could call the SSE2 (or NEON) version of fill, whereas pixman_fill_fast_path() is always C only. </pre> </blockquote> I also worried about this. The reason why I modified this to simply call more specific fast path rather than pixman_fill() is that there seem to be no cases where SSE2 or NEON fill function is called through C fast paths. I thought that fast_composite_solid_fill() would be invoked only when it is delegated from upper level of implementation, that is something like SSE2 or NEON do not have solid fill fast path. But if we use more complicated fast path mechanism in the future, I also think that this modification can cause problems. So your below suggestion seem to be more proper way of grabbing perfstat data. That is also good for simple repeat which is basically C fast path but invokes other fast paths. So I will modify this based on your nested begin/end suggestion. Thanks, Taekyun Kim </body> </html>