Updated a few bits, and made this hopefully easier to diff against master by
avoiding unnecessary indentation changes (note that diffing against the last
patchset won't work well, though).
I'm inclined to move forward on this, despite the slight drop in String
performance.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode1628
src/hydrogen.cc:1628: HValue*
HGraphBuilder::BuildComputeIntegerHash(HValue* value,
On 2015/02/23 09:26:53, Sven Panne wrote:
I think this can be expressed purely in JavaScript.
Done.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode12117
src/hydrogen.cc:12117: void
HOptimizedGraphBuilder::GenerateJSCollectionGetTable(CallRuntime* call)
{
On 2015/02/23 18:43:34, adamk wrote:
On 2015/02/23 09:26:53, Sven Panne wrote:
> We can leave it as it is for now, but I think that in the future we
will have
> something more general like %_GetInternalField.
So this is an interesting one. The Hydrogen version would actually
have no
problem with %_GetInternalField, but I ran into a lot of existing
Runtime code
that tries to be safe about accessing internal fields. Given that I
only needed
access to a single internal field in this patch, this seemed the path
of least
resistance. But I agree that the real version should probably not
expose this.
As discussed offline, it seems like this will stay as-is for now: after
we collect more examples of internal fields accessed from script, we
might be able to generalize more.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.cc#newcode12151
src/hydrogen.cc:12151: IfBuilder string_checker(this);
On 2015/02/23 19:49:06, adamk wrote:
On 2015/02/23 18:43:34, adamk wrote:
> On 2015/02/23 09:26:53, Sven Panne wrote:
> > As a general rule of thumb, we should not have any non-trivial
control flow
in
> > intrinsics. This should be expressed on the JavaScript level.
>
> Thanks, this is good feedback. "No non-trivial control flow" is a
guideline I
> didn't have in mind for these, but now I do.
One more issue here: the hash field in Strings uses an Integer32
representation.
What's your recommended way to return such a thing to JS?
Turns out this works fine if it just gets returned directly to script.
Changed to %_StringGetRawHashField.
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.h
File src/hydrogen.h (right):
https://codereview.chromium.org/947683002/diff/20001/src/hydrogen.h#newcode1852
src/hydrogen.h:1852: HValue* BuildElementIndexHash(HValue* index) {
On 2015/02/23 18:43:35, adamk wrote:
On 2015/02/23 09:26:53, Sven Panne wrote:
> When we have %_HashSeed() (for isolate()->heap()->HashSeed()), this
can be
> expressed purely in JavaScript. As it is, it is too specialized.
Yeah, I only reused this for expediency, there's no reason we couldn't
compute
Smi hashes in JS (String hashes are more complex, and I think there
are more
tradeoffs there).
I've now written a JS version of this.
https://codereview.chromium.org/947683002/
--
--
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.
