[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jarin
Scary stuff. It looks ok, but fairly brittle. Adding mstarzinger@ to have a look. https://codereview.chromium.org/1244693002/diff/20001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right):

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jochen
done, ptal https://codereview.chromium.org/1244693002/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread mstarzinger
https://codereview.chromium.org/1244693002/diff/20001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/1244693002/diff/20001/src/hydrogen-instructions.h#newcode4890 src/hydrogen-instructions.h:4890: right-representation().IsTagged()) { On

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jarin
lgtm. Even though it would be even better if - all the DCHECKs took into account the new flag. - the flag was explicitly passed on the HAdd constructor (ideally as an enum, with the default value being the normal add) and then DCHECK that the representations are in sync with the constructor

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jochen
ptal https://codereview.chromium.org/1244693002/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jarin
Looks even better, I still have two suggestions, but it is up to you. https://codereview.chromium.org/1244693002/diff/80001/src/hydrogen-instructions.h File src/hydrogen-instructions.h (right): https://codereview.chromium.org/1244693002/diff/80001/src/hydrogen-instructions.h#newcode4918

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread jarin
looks good with one final nit https://codereview.chromium.org/1244693002/diff/120001/src/mips/lithium-mips.cc File src/mips/lithium-mips.cc (right): https://codereview.chromium.org/1244693002/diff/120001/src/mips/lithium-mips.cc#newcode1625 src/mips/lithium-mips.cc:1625:

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread commit-...@chromium.org via codereview.chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1244693002/140001 https://codereview.chromium.org/1244693002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this message because you are

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread commit-...@chromium.org via codereview.chromium.org
Committed patchset #8 (id:140001) https://codereview.chromium.org/1244693002/ -- -- v8-dev mailing list v8-dev@googlegroups.com 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

[v8-dev] Re: Add support for adding an external and a tagged pointer (issue 1244693002 by joc...@chromium.org)

2015-07-20 Thread commit-...@chromium.org via codereview.chromium.org
Patchset 8 (id:??) landed as https://crrev.com/4e263bc581081c1fa925554943693f3e386fc815 Cr-Commit-Position: refs/heads/master@{#29760} https://codereview.chromium.org/1244693002/ -- -- v8-dev mailing list v8-dev@googlegroups.com http://groups.google.com/group/v8-dev --- You received this