Code is caught up to fairly current (as of Nov 5th).
I've gone through the nit comments and addressed them.
We need to discuss/agree on the conversion of #if to if as it appears to be
more
of a style issue (and may introduce a lot of changes).
Also, one small "beg for forgiveness" comment. We will add it to the list of
things to implement properly, or get back on why it doesn't work as
expected.
(it didn't, but that was a while back and may have been another bug)
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc
File src/ppc/code-stubs-ppc.cc (right):
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode1
src/ppc/code-stubs-ppc.cc:1: // Copyright 2012 the V8 project authors.
All rights reserved.
On 2014/10/20 08:28:43, danno wrote:
Here and elsewhere, please change the year to 2014
Done.
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode159
src/ppc/code-stubs-ppc.cc:159: #if V8_TARGET_ARCH_PPC64
On 2014/10/20 08:28:43, danno wrote:
Here and elsewhere where possible, can you turn this into a
if (V8_TARGET_ARCH_PPC) instead? This makes more code compile on both
platforms
and reduces the number of #ifdefs.
I understand the request, but this seems to be a style that would be
unique to PowerPC (specifically the src/ppc and src/ic/ppc directories).
I don't see any usage of this pattern in the other platform directories.
This would probably result in 250-300 change areas (1000+ lines of code
change) - I'm fine with doing it if that is what it takes, but I really
don't see much benefit.
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode301
src/ppc/code-stubs-ppc.cc:301: #endif // roohack
On 2014/10/20 08:28:43, danno wrote:
Do you still want the block above in the code?
No, removed.
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode984
src/ppc/code-stubs-ppc.cc:984: //
WriteInt32ToHeapNumberStub::GenerateFixedRegStubsAheadOfTime(isolate);
On 2014/10/20 08:28:43, danno wrote:
Really delete this line?
When we were doing this work originally including this line caused a
crash on startup.
At this point - by running without this line, it caused this code
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode984
to not be needed.
I'd like to simply beg forgiveness for this missing part of the
implementation to help us get past this code contribution. We'll take
another run at including it in the near future.
https://codereview.chromium.org/571173003/diff/20001/src/ppc/code-stubs-ppc.cc#newcode4385
src/ppc/code-stubs-ppc.cc:4385: #if ABI_USES_FUNCTION_DESCRIPTORS
On 2014/10/20 08:28:43, danno wrote:
Is it possible to use just an "if" and not "#if"?
Yes, but I've got the same comment as before. Yes, we'd run more code
through the compiler for a given bit size -- and it'd go an optimize the
always false case out, but this mostly seems like a code-style thing.
At least this one is a much smaller set of changes we're talking about.
Can you state a more specific opinion on how much this matters?
https://codereview.chromium.org/571173003/diff/20001/src/ppc/codegen-ppc.cc
File src/ppc/codegen-ppc.cc (right):
https://codereview.chromium.org/571173003/diff/20001/src/ppc/codegen-ppc.cc#newcode377
src/ppc/codegen-ppc.cc:377: // heap_number: new heap number
On 2014/10/20 08:28:43, danno wrote:
Indentation?
Done.
https://codereview.chromium.org/571173003/
--
--
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.
