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

Issue 195084: Review: compiler failed to mark a const as "not written" (Closed)

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

Description

Bug fix: When a function's 'return' statement is not the last statement in the function body, we use a trick where we wrap the function in a 'do { ... } while (0)' and the return is like a 'break'. (This is because we don't have a true jump instruction, which is a pain for SIMD.) We say 'while 0' because we don't want to LOOP, we just want a loop to jump out of, if you catch my drift. The bug is that I made the "0" a true constant, but forgot to mark that the while condition was not written by the while statement itself, and therefore got a "can't write to a constant" error later on.

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -1 line) Patch
src/liboslcomp/codegen.cpp View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 3
larrygritz
14 years, 2 months ago (2010-01-28 01:57:44 UTC) #1
clifford.stein
On this one, I'll have to take your word for it. LGTM!
14 years, 2 months ago (2010-01-28 16:50:36 UTC) #2
lg_imageworks.com
14 years, 2 months ago (2010-01-28 23:06:49 UTC) #3
Right before checking in, I noticed that the same mistake could happen for
ordinary loops:

	while (1) { }
or
	do { } while(0);

I just fixed in in both places in the identical way.


On Jan 28, 2010, at 8:50 AM, <clifford.stein@gmail.com>
<clifford.stein@gmail.com> wrote:

> On this one, I'll have to take your word for it.  LGTM!
> 
> http://codereview.appspot.com/195084/show
> 

--
Larry Gritz
lg@imageworks.com




Sign in to reply to this message.

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