On 2015/02/04 18:39:34, Benedikt Meurer wrote:
On 2015/01/30 12:36:57, dougc wrote:
> On 2015/01/30 12:15:03, Benedikt Meurer wrote:
> > > The index is constrained to be a 31-bit unsigned integer because:
it is
an
> > array buffer index, and the maximum array buffer size is 2GB, and at
this
> point
> > in the code the index is known to be within bounds?
> >
> > That's only true for asm.js heap access, not for Load/Store in
general.
>
> Could you help me understand what aspect of asm.js this is limited to?
>
> This code path is only taken for typed arrays, when
> IsExternalArrayElementsKind() is true? E.g. See the logic in
> JSTypedLowering::ReduceJSLoadProperty().
As said, that is the current implementation, which only considers asm.js.
But
we
will be targeting regular JavaScript soon(ish), and that means you can
have
abitrary Loads/Stores, i.e. even Loads/Stores for Blink DOM objects or
other
arrays/objects/whatever. You cannot make any assumptions about the index
in
that
case.
It's true that asm.js heap access patterns are a subset of the code this
path
supports, but this path is not limited to asm.js, rather it deals with typed
array element access. For example u32[i+K] is supported by this path but is
not
asm.js.
It would appear that LoadElement() and StoreElement() are only used when the
index is within bounds, as they do no bounds checking, so even if these were
used for other objects they would only be used when the index is known to be
within bounds of the object? Further ReduceJSLoadProperty() would be
checking
that the index is within the 2G limit. Thus even in future the index will
be an
unsigned 31-bit integer if the LoadElement() or StoreElement() paths are
taken?
Transforming the addition to a 64 bit operation earlier might also frustrate
later optimizations. For example the right and left shift are optimized
away via
some patterns etc, and the addition moved over a low bit masking operation.
A 64
bit op might require more general pattern matching and might not even
capture
the constrains needed to satisfy the transforms. It might be sound to lower
to
32 bit ops when possible.
If the code is in flux then I can wait and rework the patch in future.
https://codereview.chromium.org/860283004/
--
--
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.
