Re: Review: improve adjust_varying (issue204046)


Larry Gritz <l...@...>
 

On Feb 5, 2010, at 8:04 PM, <solomo...@...> <solomo...@...> wrote:

http://codereview.appspot.com/204046/diff/1/6#newcode704
src/liboslexec/exec.cpp:704: varying_assignment |= !
running_top_level();
running_top_level() suggests that you can't be inside any conditional
(as does the comment above). Of course the real logic is that you can't
have "diverged". Why not express it in terms of has_not_diverged()?
Yeah, I suppose that's more clear. Chris? Anybody else? Opinions?


http://codereview.appspot.com/204046/diff/1/4
File src/liboslexec/opcontrol.cpp (right):

http://codereview.appspot.com/204046/diff/1/4#newcode172
src/liboslexec/opcontrol.cpp:172: // From here on, varying condition or
potential
Like the if clause above, it seems that we should be checking to see
whether or not the condition is actually identical for all values. That
would mean that it isn't really "varying", we just didn't notice... The
same thing holds for dowhile below.
I'll put in a FIXME for now.

--
Larry Gritz
l...@...

Join osl-dev@lists.aswf.io to automatically receive all group messages.