Reverted the changes in compiler.cc, so that the coverage of the full
compiler
on all three platforms is the same and unchanged by this change.
I will make a follow up change where the full compiler on ia32 will take
full
effect when debugging is enabled.
http://codereview.chromium.org/1989012/diff/9001/10001
File src/arm/full-codegen-arm.cc (right):
http://codereview.chromium.org/1989012/diff/9001/10001#newcode1105
src/arm/full-codegen-arm.cc:1105: __ push(r0);
On 2010/05/12 12:03:52, fschneider wrote:
Could you use the stm instruction here for more compact code?
Done (used Push(cp, r0)).
http://codereview.chromium.org/1989012/diff/9001/10003
File src/compiler.cc (right):
http://codereview.chromium.org/1989012/diff/9001/10003#newcode136
src/compiler.cc:136: #ifdef V8_TARGET_ARCH_IA32
On 2010/05/12 12:03:52, fschneider wrote:
Maybe there is no need for this #ifdef? The FastCodeGen is hidden
behind a flag
and not enabled by default. We could also consider removing it
completely for
now (as a separate change).
Removed, that was a mistake. I did not intend to mess with the fast
compiler.
http://codereview.chromium.org/1989012/diff/9001/10004
File src/full-codegen.cc (right):
http://codereview.chromium.org/1989012/diff/9001/10004#newcode58
src/full-codegen.cc:58: #ifndef V8_TARGET_ARCH_IA32
On 2010/05/12 12:03:52, fschneider wrote:
Maybe move this #ifndef to the call site of the syntax checker (until
the other
platforms are completed)?
Done.
http://codereview.chromium.org/1989012/diff/9001/10006
File src/ia32/full-codegen-ia32.cc (right):
http://codereview.chromium.org/1989012/diff/9001/10006#newcode2568
src/ia32/full-codegen-ia32.cc:2568: __
CallRuntime(Runtime::kSwapElements, 3);
On 2010/05/12 11:02:20, antonm wrote:
runtime call here might be quite expensive. I don't have any numbers,
but when
x64 port had only runtime call w/o inlined version, there was a
notable
regression.
We have revised the current scope of the full scope of the full
compiler, so that it will only be in full effect when debugging
otherwise it will keep the scope currently defined by the syntex
checker. Therefore having a runtime call here should be ok.
http://codereview.chromium.org/1989012/diff/9001/10006#newcode2620
src/ia32/full-codegen-ia32.cc:2620: // it make sense to use that here
instead of the runtime call?
On 2010/05/12 11:02:20, antonm wrote:
runtime call might be fine most of the time. The only problem: if we
have cache
miss invocation of JS function from C++ might be unnecessary
expensive. I think
that the stub is the best solution. I didn't implement it in the
first place to
keep pieces digestible.
See comment above. Keeping runtime call.
http://codereview.chromium.org/1989012/diff/9001/10006#newcode2624
src/ia32/full-codegen-ia32.cc:2624: STATIC_ASSERT(kSmiTag == 0);
On 2010/05/12 11:02:20, antonm wrote:
if you're going to uncomment this code, those assertions are probably
unnecessary
This code will be removed.
http://codereview.chromium.org/1989012/show
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
