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

Issue 4899047: Add an compile option to unroll for-loops with integer indices. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 3 months ago by zmo
Modified:
13 years, 3 months ago
Reviewers:
kbr1, kbr
CC:
angleproject-review_googlegroups.com
Base URL:
http://angleproject.googlecode.com/svn/trunk/
Visibility:
Public.

Description

Add an compile option to unroll for-loops with integer indices. ANGLEBUG=193 TEST=with this option, for-loops with integer indices will be unrolled. Committed: http://code.google.com/p/angleproject/source/detail?r=734

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+58 lines, -5 lines) Patch
M include/GLSLANG/ShaderLang.h View 1 chunk +2 lines, -1 line 0 comments Download
M samples/translator/translator.cpp View 2 chunks +3 lines, -1 line 0 comments Download
M src/build_angle.gyp View 2 chunks +2 lines, -2 lines 0 comments Download
M src/common/version.h View 1 chunk +1 line, -1 line 0 comments Download
M src/compiler/Compiler.cpp View 2 chunks +5 lines, -0 lines 0 comments Download
M src/compiler/ForLoopUnroll.h View 1 chunk +3 lines, -0 lines 0 comments Download
M src/compiler/ForLoopUnroll.cpp View 2 chunks +42 lines, -0 lines 1 comment Download

Messages

Total messages: 3
zmo
Please review.
13 years, 3 months ago (2011-08-16 22:40:03 UTC) #1
kbr1
LGTM -- how was this tested? One minor comment. http://codereview.appspot.com/4899047/diff/1/src/compiler/ForLoopUnroll.cpp File src/compiler/ForLoopUnroll.cpp (right): http://codereview.appspot.com/4899047/diff/1/src/compiler/ForLoopUnroll.cpp#newcode146 src/compiler/ForLoopUnroll.cpp:146: ...
13 years, 3 months ago (2011-08-17 18:55:38 UTC) #2
zmo
13 years, 3 months ago (2011-08-17 19:12:42 UTC) #3
On 2011/08/17 18:55:38, kbr1 wrote:
> LGTM -- how was this tested? One minor comment.

Tested manually with a few shaders with loops.

> 
> http://codereview.appspot.com/4899047/diff/1/src/compiler/ForLoopUnroll.cpp
> File src/compiler/ForLoopUnroll.cpp (right):
> 
>
http://codereview.appspot.com/4899047/diff/1/src/compiler/ForLoopUnroll.cpp#n...
> src/compiler/ForLoopUnroll.cpp:146: void
> ForLoopUnroll::MarksUnrollForLoopWithIntegerIndex(TIntermNode* root)
> The naming convention (MarksUnrollForLoopWithIntegerIndex vs. the class
> MarkUnrollForLoopWithIntegerIndex -- one letter difference) is confusing.
Could
> you make this method a verb (MarkForLoopsWithIntegerIndicesForUnrolling?) and
> the class above a noun (maybe even a briefer abbreviation,
> IntegerForLoopUnrollMarker)?

Will do.
Sign in to reply to this message.

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