Stephen, we also have this same constant folding in the hydrogen->lithium translation, with the drawback that it generates dead code for the not-taken branch and a jump around it on one side or the other. I'd be interested in knowing if it's this dead code and jump you want to get rid of, or if we have a case where that's really not working for us and we're testing the value of the constant.
On Fri, Jun 10, 2011 at 12:05 PM, <[email protected]> wrote: > > http://codereview.chromium.**org/7044100/diff/1/src/parser.**cc<http://codereview.chromium.org/7044100/diff/1/src/parser.cc> > File src/parser.cc (right): > > http://codereview.chromium.**org/7044100/diff/1/src/parser.** > cc#newcode2559<http://codereview.chromium.org/7044100/diff/1/src/parser.cc#newcode2559> > src/parser.cc:2559: if (value == 0) { > On 2011/06/10 09:20:08, Kasper Lund wrote: > >> Could this be something ala: >> > > Handle<Object> boolean(isolate->heap()->**ToBoolean(value == 0)); >> return new(zone()) Literal(boolean); >> > > or perhaps consider negating the result of calling >> expression->AsLiteral()->**handle()->ToBoolean() and refactoring the >> > code so it > >> works for cases where the expression is a non-numeric literal. >> > > We already have this constant folding in void > FullCodeGenerator::**TestContext::Plug(Handle<**Object> lit). It just > needs > to be 'ported' to TestContext::BuildBranch(**HValue* value) in > hydrogen.cc. > > I would prefer that solution rather than having it here, because it can > properly cope with things like > > x = 0 > if (!x) ... > > > http://codereview.chromium.**org/7044100/<http://codereview.chromium.org/7044100/> > -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
