Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(452)

Issue 171055: More SSE2ification (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 11 months ago by Stephen White
Modified:
13 years, 11 months ago
Reviewers:
reed
Base URL:
http://skia.googlecode.com/svn/trunk/
Visibility:
Public.

Description

SSE2-ified S32_alpha_D32_filter_DX (refactoring to come). Also shaved a few cycles off the SSE2 blends.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+133 lines, -8 lines) Patch
src/core/SkBitmapProcState.h View 1 chunk +2 lines, -0 lines 0 comments Download
src/opts/SkBitmapProcState_opts_SSE2.h View 1 chunk +3 lines, -0 lines 0 comments Download
src/opts/SkBitmapProcState_opts_SSE2.cpp View 1 chunk +116 lines, -0 lines 0 comments Download
src/opts/SkBlitRow_opts_SSE2.cpp View 2 chunks +10 lines, -8 lines 0 comments Download
src/opts/opts_check_SSE2.cpp View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 4
Stephen White
14 years, 11 months ago (2009-12-10 02:24:26 UTC) #1
reed
LGTM Do you think this technique of pushing various function declaration into procstate.h is the ...
14 years, 11 months ago (2009-12-10 19:39:35 UTC) #2
Stephen White
On 2009/12/10 19:39:35, reed wrote: > LGTM > > Do you think this technique of ...
14 years, 11 months ago (2009-12-10 19:58:15 UTC) #3
reed
14 years, 11 months ago (2009-12-10 21:14:39 UTC) #4
OK. I don't have a better solution in mind, so we'll just keep up your pattern.

I just submitted the CL to enable the new builder by default (left the
old code-path in there for now. If it sticks, we can clean it up
later). I also uploaded new gm images to match. rev. 455

On Thu, Dec 10, 2009 at 2:58 PM,  <senorblanco@chromium.org> wrote:
> On 2009/12/10 19:39:35, reed wrote:
>>
>> LGTM
>
>> Do you think this technique of pushing various function declaration
>
> into
>>
>> procstate.h is the right pattern? It is working, but I wonder if my
>
> notion that
>>
>> the function names could be private, and only live in the table is
>
> misguided.
>
> The main thing I wanted to avoid was duplicating the tests in
> SkBitmapProcState::chooseProcs().
>
> It could be done using the table approach, we'd just have to pass an
> index value, as the BlitRow PlatformProcs() functions do.  The only
> downside is that the index is kind of anonymous, and relies on setting
> up a table that matches the one in the src dir.  At least when there are
> only a few functions, checking them by name (ptr value) is clearer in
> the code, IMHO.
>
> http://codereview.appspot.com/171055
>
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b