PTAL.
https://codereview.chromium.org/700963002/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/700963002/diff/1/src/ast.h#newcode427
src/ast.h:427: uint32_t bit_field_;
On 2014/11/05 09:11:49, Sven Panne wrote:
On 2014/11/05 01:33:17, brucedawson wrote:
> Why use a bit field at all? byte/bool/bool take up three bytes, pads
to four
> bytes, which should be perfectly space efficient, more run-time
efficient.
All the code in ast.h *must* stay as it is, for the reason see:
https://code.google.com/p/chromium/issues/detail?id=417697. This part
of the CL
would basically revert all the goodness already done.
Quick explanation: The layout of the AST nodes is carefully tuned for
space
efficiency because asm.js code results in *huge* ASTs. As it is,
to_boolean_types consumes one byte, is_parenthesized and
is_multiparenthesized
go into the next byte. The important thing: The most common node
VariableProxy
starts with 3 1bit fields, which go into the next byte. (With clang at
least,
use "clang++ -c -Xclang -fdump-record-layouts" to see what really
happens)
If we blindingly change the layout here, we will blow away hundreds of
megabytes
for asm.js stuff. Yes, this is all very fragile, but it's important
nevertheless.
Perhaps we should have a look at the layout of the nodes under VC, but
I don't
know how to do that. Hints? :-)
Used a smaller field per offline discussion.
https://codereview.chromium.org/700963002/diff/1/src/hydrogen.h
File src/hydrogen.h (right):
https://codereview.chromium.org/700963002/diff/1/src/hydrogen.h#newcode222
src/hydrogen.h:222: uint32_t bit_field_;
On 2014/11/05 09:11:49, Sven Panne wrote:
Can we leave this as it is? It's only a sequence of booleans, which is
OK, and
furthermore HBasicBlock is not relevant for any kind of space savings.
The
BitField stuff is only obfuscating things here.
Done. (Though I don't agree with the term "obfuscating"; yes it's more
verbose, but also more consistent and explicit/reliable which IMHO
justifies the change. But whatever.)
https://codereview.chromium.org/700963002/diff/1/src/hydrogen.h#newcode1716
src/hydrogen.h:1716: uint32_t bit_field_;
On 2014/11/05 09:11:49, Sven Panne wrote:
Same here: This is obfuscating things, and IfBuilder is not relevant
for space
efficiency.
Done.
https://codereview.chromium.org/700963002/diff/1/src/scanner.cc
File src/scanner.cc (right):
https://codereview.chromium.org/700963002/diff/1/src/scanner.cc#newcode249
src/scanner.cc:249: if (c0_ < 0) break;
On 2014/11/05 09:11:49, Sven Panne wrote:
How is this related to the bitfield fiddling?
As the comment says, the unicode_cache_ only accepts unsigned inputs.
Since it's now using BitField, it trips over a DCHECK that the
implicitly-converted -1 (which signals EOS) can actually be stored.
https://codereview.chromium.org/700963002/
--
--
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.
