https://codereview.chromium.org/176843006/diff/40001/src/types.h
File src/types.h (right):
https://codereview.chromium.org/176843006/diff/40001/src/types.h#newcode144
src/types.h:144: V(TaggedPtr, -1 << 30 | kSemantic) /* MSB has
to be sign-extended */ \
On 2014/03/14 10:37:37, rossberg wrote:
On 2014/03/05 07:22:36, Sven Panne wrote:
> On 2014/03/04 15:02:25, Michael Starzinger wrote:
> > I would be fine with this. If either Benedikt or Sven want this to
be
handled
> > differently, then I defer to alternative suggestions from their
end.
>
> As already discussed yesterday, I consider this sign-extension here
an
extremely
> ugly and confusing hack. We just want to stuff something which the
GC
shouldn't
> care about into some kind of JavaScript value (a Smi, but that't
purely
> accidental here). We do similar things e.g. in the exterrnal API,
and I don't
> see why something similar to DecodeSmiToAligned/EncodeAlignedAsSmi
shouldn't
> work with the bit set at hand. As it is, the current CL is mixing
unrelated
> things together.
Michael yesterday (after you had left) expressed very strong
disagreement with
your suggestion, and the more I think about the more I agree with him.
There are two ways to implement your suggestion: direct bit fiddling
with smi
tags and shift offsets, which is platform dependent and pierces the
little
abstraction barrier we have. I hope we can agree that that's a no-go
(even
though that's what DecodeSmiToAligned is using, which I consider the
real
mistake around here).
Or you extend the Smi class interface to support encoding/decoding
unsigneds.
That, however, severely weakens the Smi class, which Michael is
strongly opposed
to. In particular, the 'value' method, which currently is the only and
always
correct way to extract the value, would cease to have this property.
You would
have to start relying on global knowledge to know which projection
method to
use, and we have enough of such ambient "invariants" already. So this
is also
bad.
The above solution, on the other hand, is entirely local. All it does
is solving
a simple isolated problem: how to encode an 31-bit bitfield into an
31-bit
signed integer. This is completely independent of their later
injection into
smis and their low-level encoding details (which is all the more
relevant since
smis are just one possible backing store for these integers, in zone
types it's
a different one). Separation of concerns, pal!
So just to clarify: My strong disagreement is against having something
like Smi::FromUsigned() or Smi::unsigned_value() because that weakens a
Smi to basically be either a "Smsi" or "Smui" depending on which methods
are used. And I think that is a bad direction to be heading.
I know that we already use trickery to cast to and from Smi (e.g.
EncodeAlignedAsSmi and DecodeSmiToAligned), but at least those methods
are not part of the "Smi API". Performance forces us to use these hacks,
so I won't start arguing against them. But where I draw the line and
will start arguing is at extending the Smi class in a way that invites
everyone to use hacks like these. Such an API would invite accidental
misuse (e.g. creating a Smi with Smi::FromUnsigned, reading back the
value with Smi::value).
https://codereview.chromium.org/176843006/
--
--
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.
