After some testing, I'll upload a new version with the changes...
http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.cc File src/ia32/code-stubs-ia32.cc (right): http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.cc#newcode719 src/ia32/code-stubs-ia32.cc:719: On 2011/04/26 13:32:52, fschneider wrote:
Remove one extra line.
Done. http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h File src/ia32/code-stubs-ia32.h (right): http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h#newcode143 src/ia32/code-stubs-ia32.h:143: void GenerateHeapNumberCodeSub(MacroAssembler* masm, // Label* undo, On 2011/04/26 13:32:52, fschneider wrote:
Remove commented code + fit on one line.
Done. http://codereview.chromium.org/6879081/diff/10001/src/ia32/code-stubs-ia32.h#newcode145 src/ia32/code-stubs-ia32.h:145: void GenerateHeapNumberCodeBitNot(MacroAssembler* masm, // Label* undo, On 2011/04/26 13:32:52, fschneider wrote:
Remove commented code + fit on one line.
Done. http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc File src/ia32/full-codegen-ia32.cc (right): http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3744 src/ia32/full-codegen-ia32.cc:3744: // TODO(svenpanne): Allowing format strings in Comment would be nice here... On 2011/04/26 13:32:52, fschneider wrote:
It's a little complicated to do, since format strings require
allocating a
buffer in JS heap since comments are referenced by the reloc-info... -
so I'm
not sure if you should make it a TODO now.
I thought about using something like vsnprintf in the Comment constructor, this shouldn't be too hard, I think or am I missing something here? http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3752 src/ia32/full-codegen-ia32.cc:3752: VisitForAccumulatorValue(expr->expression()); On 2011/04/26 13:32:52, fschneider wrote:
You need to call here:
SetSourcePosition(expr->position())
This will record the statement position which we use to associate the
type
feedback with. We have the same call to SetSourcePosition for binary
operations. Done, for x64 and ARM, too. Why is Expression::position() not pure? Using UNREACHABLE just hides semantic errors when a subclass doesn't properly implement this. Another possibility might be moving the pos_ field to Expression itself, making position() a "normal" method there. http://codereview.chromium.org/6879081/diff/10001/src/ia32/full-codegen-ia32.cc#newcode3753 src/ia32/full-codegen-ia32.cc:3753: __ CallStub(&stub); On 2011/04/26 13:32:52, fschneider wrote:
You should use here:
EmitCallIC(stub.GetCode(), NULL)
where NULL signals that there is no inlined smi code. Since we probably want to add patchable inline smi code later (like we
already
do for binary ops)
I have to admit that I still don't understand this kind of patching in detail, but I've changed this like you said. :-) Of course, the x64 and ARM parts have been changed accordingly, too. Note that IMHO this triple change is a strong hint that there is a bit too much copy-n-paste in the code... http://codereview.chromium.org/6879081/diff/10001/src/ic.cc File src/ic.cc (right): http://codereview.chromium.org/6879081/diff/10001/src/ic.cc#newcode1819 src/ic.cc:1819: case SMI: return "SMI"; On 2011/04/26 13:32:52, fschneider wrote:
--> "Smi"
Done. http://codereview.chromium.org/6879081/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
