http://codereview.chromium.org/8373029/diff/32026/src/ast.h File src/ast.h (right):
http://codereview.chromium.org/8373029/diff/32026/src/ast.h#newcode713 src/ast.h:713: bool IsSymbolCompare() { return compare_type_ == SYMBOL_ONLY; } On 2011/11/01 09:26:19, fschneider wrote:
You introduce IsSymbolCompare, but it does not seem to be used
anywhere. Can you
use it in the same way we use the type feedback for smis?
Probably, better remove it, as I can't see any use for it in current version of code (because of oddball checks) http://codereview.chromium.org/8373029/diff/32026/src/hydrogen.cc File src/hydrogen.cc (right): http://codereview.chromium.org/8373029/diff/32026/src/hydrogen.cc#newcode2786 src/hydrogen.cc:2786: compare = new(zone()) HCompareGenericAndBranch(context, tag_value, On 2011/11/01 09:26:19, fschneider wrote:
Not sure this works: If you pass an object as tag value, we may get a lazy-deoptimization at this instruction: The generic compare may
invoke other JS
code through valueOf method.
The problem is that we can't insert a LLazyBailout instruction for the lazy-deopt after a branch. That's why we only had CompareGeneric
(without
branch).
How about only handling the string-cases, and deoptimize on others? I
think the
code would get a lot simpler, too.
tag_value can't be object, as we have oddball check above http://codereview.chromium.org/8373029/diff/32026/src/ia32/lithium-ia32.cc File src/ia32/lithium-ia32.cc (right): http://codereview.chromium.org/8373029/diff/32026/src/ia32/lithium-ia32.cc#newcode1464 src/ia32/lithium-ia32.cc:1464: return AssignEnvironment(MarkAsCall(DefineFixed(result, eax), instr)); On 2011/11/01 09:26:19, fschneider wrote:
MarkAsCall should already assign environments if necessary, so no need
to
explicitly do it here.
It was segfaulting w/o that http://codereview.chromium.org/8373029/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
