I've got one real concern (in code-stubs-hydrogen.cc), the rest is not
really
related to this CL, but related to the fact that our #include structure is
extremely bad: Looking at all the place which had to be modified (I stopped
commenting on them after a few), we should really restructure this. There
are
various way of doing this much cleaner (e.g. by having the build system
pass the
right #include paths or by doing the #includes the other way round). This
should
not be part of this CL, it's just a reminder. The more platform ports we
have in
our tree, the more we should care about doing them right...
https://codereview.chromium.org/293743005/diff/1/src/assembler.cc
File src/assembler.cc (right):
https://codereview.chromium.org/293743005/diff/1/src/assembler.cc#newcode69
src/assembler.cc:69: #elif V8_TARGET_ARCH_X87
Not really related to this CL, but...: I think that we should really
reconsider the structure of our #includes regarding the assembler. Doing
it this way is really upside-down...
https://codereview.chromium.org/293743005/diff/1/src/code-stubs-hydrogen.cc
File src/code-stubs-hydrogen.cc (right):
https://codereview.chromium.org/293743005/diff/1/src/code-stubs-hydrogen.cc#newcode336
src/code-stubs-hydrogen.cc:336: #if !defined(V8_TARGET_ARCH_X87)
On 2014/05/20 05:44:02, Weiliang wrote:
x87 is more easy to spill the double register, so disable it.
Please don't introduce any platform-specific #ifdefs into the
non-platform specific parts. I know that we already have a few of them,
but we really need to get rid of them, and don't introduce any more of
them. In my POV, this is a blocker for this CL.
https://codereview.chromium.org/293743005/diff/1/src/code-stubs.h
File src/code-stubs.h (right):
https://codereview.chromium.org/293743005/diff/1/src/code-stubs.h#newcode449
src/code-stubs.h:449: #elif V8_TARGET_ARCH_X87
Same comment as for the assembler...
https://codereview.chromium.org/293743005/diff/1/src/codegen.h
File src/codegen.h (right):
https://codereview.chromium.org/293743005/diff/1/src/codegen.h#newcode59
src/codegen.h:59: #include "x87/codegen-x87.h"
... and again the same comment here. :-/
https://codereview.chromium.org/293743005/diff/1/src/frames-inl.h
File src/frames-inl.h (right):
https://codereview.chromium.org/293743005/diff/1/src/frames-inl.h#newcode22
src/frames-inl.h:22: #elif V8_TARGET_ARCH_X87
... and one more time: Upside-down #includes. :-(
https://codereview.chromium.org/293743005/diff/1/src/frames.cc
File src/frames.cc (right):
https://codereview.chromium.org/293743005/diff/1/src/frames.cc#newcode453
src/frames.cc:453: #if V8_TARGET_ARCH_IA32 || V8_TARGET_ARCH_X87
Again not related to this CL: We should make the decision if we need
some alignment bit fiddling below at *construction time* of StackFrame
and remove the #if.
https://codereview.chromium.org/293743005/diff/1/src/platform-posix.cc
File src/platform-posix.cc (right):
https://codereview.chromium.org/293743005/diff/1/src/platform-posix.cc#newcode445
src/platform-posix.cc:445: #if V8_TARGET_ARCH_IA32 || V8_TARGET_ARCH_X87
Not related to this CL: We should re-evaluate if all the stuff below is
really necessary nowadays.
https://codereview.chromium.org/293743005/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
