On 2015/01/29 09:54:33, Sven Panne wrote:
Almost there... (Don't forget to rebase before upload, we're changing
quite
fast.)
https://codereview.chromium.org/866843003/diff/1/src/assembler.cc
File src/assembler.cc (right):
https://codereview.chromium.org/866843003/diff/1/src/assembler.cc#newcode1461
src/assembler.cc:1461: #elif V8_OS_AIX
We don't test MINGW builds, either, it was just part of an external
contribution
IIRC. Just use your AIX code for the MINGW stuff above, too, and let's
see if
somebody complains. >:-) Having weird conditional untested code around is
definitely worse.
https://codereview.chromium.org/866843003/diff/1/src/base/logging.h
File src/base/logging.h (right):
https://codereview.chromium.org/866843003/diff/1/src/base/logging.h#newcode80
src/base/logging.h:80: #if V8_OS_AIX && V8_HOST_ARCH_32_BIT
The whole Check(Non)EqualsHelper approach in v8 is fundamentally broken:
The
expected value and the actual value have to be of the same type, and this
gives
endless headaches with C++'s integral promotion rules when you mix
different
sizes/signedness. We should really just use Chrome's approach where this
is
handled via templates with 2 type arguments, there is no other sane way.
Benedikt just told me that he wants to give it try, but this shouldn't
block
this CL: I propose to leave your code here as it is in the CL, we can
clean
this
up later when we have a sane framework.
https://codereview.chromium.org/866843003/diff/1/src/base/platform/platform-aix.cc
File src/base/platform/platform-aix.cc (right):
https://codereview.chromium.org/866843003/diff/1/src/base/platform/platform-aix.cc#newcode46
src/base/platform/platform-aix.cc:46: // converted to double precision
numbers.
On 2015/01/29 00:08:29, michael_dawson wrote:
> [...] Having discussed this with the team we'll remove as the right
answer
is
more likely to
> fix the test or exclude the test than to try and force the addresses we
use.
Exactly, this seems to be the right approach. Let's fix the root cause and
don't
silence the symptoms in an ugly way. :-)
You can just blacklist the test on AIX for now and open a v8 issue for it.
Then
I can try to find somebody who understands the test and can fix it, I
don't
even
have the slightest clue what EquivalenceOfLoggingAndTraversal is trying to
test
and what it is doing in detail.
https://codereview.chromium.org/866843003/diff/1/src/d8.cc
File src/d8.cc (right):
https://codereview.chromium.org/866843003/diff/1/src/d8.cc#newcode66
src/d8.cc:66: #define malloc __linux_malloc
Hmmm, if I see this correctly, malloc(0) only happens via the
MockArrayBufferAllocator. I would prefer to leave out this #ifdef and
change
MockArrayBufferAllocator to allocate a single byte instead. This should
hopefully work, if not, we learn something. :-)
As a side note, this would probably be more correct, anyway: POSIX
explicitly
states that malloc(0) has implementation-defined behavior (either
returning
NULL
or a unique pointer).
https://codereview.chromium.org/866843003/diff/1/test/mjsunit/mjsunit.status
File test/mjsunit/mjsunit.status (right):
https://codereview.chromium.org/866843003/diff/1/test/mjsunit/mjsunit.status#newcode251
test/mjsunit/mjsunit.status:251: 'asm/embenchen/*': [SKIP],
*sigh* OK, just leave the SKIP here, Wintel legacy strikes again...
In terms of re-basing, I've been addressing the comments, re-basing, running
make quickcheck and then uploading. On my current machine quickcheck is a
5-6
hour run so the re-base will always be a number of hours old.
https://codereview.chromium.org/866843003/
--
--
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.
