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

Issue 448041: Rutime code optimization (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
14 years, 1 month ago by larrygritz
Modified:
14 years, 1 month ago
Reviewers:
ckulla
CC:
dev-osl_imageworks.com, osl-dev_googlegroups.com
Base URL:
http://openshadinglanguage.googlecode.com/svn/trunk/
Visibility:
Public.

Description

First stab at runtime optimization of OSL bytecode, based on the actual instance parameters. This is a pretty big refactor, in that it moves quite a bit of compiler infrastructure (like lifetime analysis and temporary coalescing) to runtime rather than in oslc. The basic idea is that we allow a hint in the shader parameter metadata, [[ int lockgeom=1|0 ]] that says whether or not that parameter is "locked" with respect to the geometry (if locked, its value cannot be overridden by interpolated geometric primitive variables). This means that by the time we shade, many of the parameters (that are not connected or overridden on the geom) have known values, and we can constant-fold the crap out of the code for each instance. For example, multiplication by 1 is eliminated, "if (foo)" can end up removing large swaths of code if foo's value is known, etc. I've implemented only a very few obvious optimizations, but it's already showing a 25% speedup overall in our renderer (even counting the extra runtime we spend optimizing shader code). I expect this to improve speed even more dramatically as I continue to add other optimizations as well as make the analysis more sophisticated (for example, it currently doesn't consider connections at all, even though it could often know the connected value and treat that, too, as a constant). I'm aware that many parts of this are rough around the edges. It's a work in progress. But the basic infrastructure is working, and there's a definite speed gain, so I'd like to commit this even though it's an ongoing project. Then subsequent additional optimizations and refactorings will be smaller changes that we can do incrementally. There are new ShadingSystem options "lockgeom" that gives the default value for "lockgeom" (so you can "opt in" or "opt out", depending on your taste), and for the optimization level to perform at runtime.

Patch Set 1 #

Patch Set 2 : Minor bug fix: ShaderGroup::clear needed to set m_optimized=false #

Total comments: 10
Unified diffs Side-by-side diffs Delta from patch set Stats (+1503 lines, -111 lines) Patch
src/include/osl_pvt.h View 1 5 chunks +22 lines, -5 lines 4 comments Download
src/liboslcomp/codegen.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
src/liboslcomp/oslcomp.cpp View 1 8 chunks +35 lines, -19 lines 1 comment Download
src/liboslcomp/oslcomp_pvt.h View 1 3 chunks +24 lines, -3 lines 0 comments Download
src/liboslcomp/symtab.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
src/liboslcomp/symtab.cpp View 1 1 chunk +1 line, -1 line 0 comments Download
src/liboslexec/CMakeLists.txt View 1 1 chunk +4 lines, -1 line 1 comment Download
src/liboslexec/context.cpp View 1 1 chunk +7 lines, -0 lines 0 comments Download
src/liboslexec/exec.cpp View 1 12 chunks +20 lines, -14 lines 1 comment Download
src/liboslexec/instance.cpp View 1 8 chunks +139 lines, -20 lines 3 comments Download
src/liboslexec/loadshader.cpp View 1 3 chunks +9 lines, -1 line 0 comments Download
src/liboslexec/master.cpp View 1 2 chunks +0 lines, -14 lines 0 comments Download
src/liboslexec/opstring.cpp View 1 2 chunks +4 lines, -8 lines 0 comments Download
src/liboslexec/oslexec_pvt.h View 1 18 chunks +86 lines, -18 lines 0 comments Download
src/liboslexec/osolex.l View 1 1 chunk +6 lines, -0 lines 0 comments Download
src/liboslexec/runtimeoptimize.cpp View 1 1 chunk +1116 lines, -0 lines 0 comments Download
src/liboslexec/shadingsys.cpp View 1 6 chunks +22 lines, -4 lines 0 comments Download
src/testshade/testshade.cpp View 1 4 chunks +5 lines, -2 lines 0 comments Download

Messages

Total messages: 11
larrygritz
14 years, 1 month ago (2010-03-11 19:40:31 UTC) #1
larrygritz
Minor bug fix: ShaderGroup::clear needed to set m_optimized=false
14 years, 1 month ago (2010-03-12 18:55:19 UTC) #2
ckulla
http://codereview.appspot.com/448041/diff/3001/4001 File src/include/osl_pvt.h (right): http://codereview.appspot.com/448041/diff/3001/4001#newcode632 src/include/osl_pvt.h:632: bool is_const () const { return symtype() == SymTypeConst; ...
14 years, 1 month ago (2010-03-12 23:58:30 UTC) #3
ckulla
http://codereview.appspot.com/448041/diff/3001/4001 File src/include/osl_pvt.h (right): http://codereview.appspot.com/448041/diff/3001/4001#newcode817 src/include/osl_pvt.h:817: unsigned int argread_bits () const { return m_argread; } ...
14 years, 1 month ago (2010-03-12 23:59:45 UTC) #4
lg_imageworks.com
On Mar 12, 2010, at 3:58 PM, <ckulla@gmail.com> wrote: > > http://codereview.appspot.com/448041/diff/3001/4001 > File src/include/osl_pvt.h ...
14 years, 1 month ago (2010-03-13 00:44:04 UTC) #5
ckulla
> > It's been SymTypeConst for a long time. If you feel strongly about it, ...
14 years, 1 month ago (2010-03-13 00:55:50 UTC) #6
lg_imageworks.com
On Mar 12, 2010, at 4:55 PM, <ckulla@gmail.com> wrote: >>> http://codereview.appspot.com/448041/diff/3001/4014#newcode374 >>> src/liboslexec/exec.cpp:374: sym.initialized (true); ...
14 years, 1 month ago (2010-03-13 01:09:39 UTC) #7
lg_imageworks.com
On Mar 12, 2010, at 5:09 PM, Larry Gritz wrote: > On Mar 12, 2010, ...
14 years, 1 month ago (2010-03-13 01:12:33 UTC) #8
ckulla
> To elaborate further: correct code generation shouldn't yield this case. Every > once in ...
14 years, 1 month ago (2010-03-13 01:16:00 UTC) #9
larrygritz
So... where are we? LGTY, or are there additional concerns for this first checkin?
14 years, 1 month ago (2010-03-13 07:44:31 UTC) #10
ckulla
14 years, 1 month ago (2010-03-15 17:49:00 UTC) #11
On 2010/03/13 07:44:31, larrygritz wrote:
> So... where are we?  LGTY, or are there additional concerns for this first
> checkin?

LGTM!
Sign in to reply to this message.

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