https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h
File src/arm64/simulator-arm64.h (right):


https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode165
src/arm64/simulator-arm64.h:165: ASSERT(size/8 <= kXRegSize);
On 2014/03/27 17:50:42, jbramley wrote:
> There are actually only two supported sizes at the moment; T in the original
may
> have had other sizes of course, but the _access_ is always 32- or 64-bits.
> Perhaps this should assert that size is either kXRegSizeInBits or
> kWRegSizeInBits (or kDRegSizeInBits or kSRegSizeInBits).
>
> The use of kSizeInBytes in the original was to allow for NEON Q registers in
the
> future; this would require a 128-bit backing store and the ability to store
128
> bits at a time.

Is there a plan to use Q registers? I think this method can safely extend to
128-bit registers.  uint128_t is available in gcc 4.8.  Not sure what
compilers
are required to be supported for the compiler.

ARM has them, and whatever optimisation they're used for there ought to be
applicable to A64 too.

I don't know, but I suspect that we need to be able to support much older GCCs
than that. My stdint.h doesn't have int128_t, but GCC 4.8 seems to have a
built-in __int128_t type. However, support for __int128_t is also
target-specific, so there's a good chance that it won't be available for A64
anyway.


https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode179
src/arm64/simulator-arm64.h:179: } reg = { new_value };
On 2014/03/27 17:50:42, jbramley wrote:
> This isn't safe for type-aliasing rules. In another recent patch, we decided
not
> to worry about that, but it's worth noting it here.
>
> On the other hand, a memcpy here should be free if the size is known at
compile
> time. Perhaps we could keep this type-aliasing-safe and still have the speed
> advantage:
>
> void Set(T new_value) {
>   value_ = 0;
>   memcpy(&value_, &new_value, sizeof(new_value));
> }
>
> void Set(T new_value, unsigned size) {
>   if (size == sizeof(new_value)) {
>     Set(new_value);
>   } else {
>     // Slower case.
>   }
> }

I tried this method earlier.
https://codereview.chromium.org/203263017/
and it had to be reverted due to compiler bugs. I think this method is closer
to what would be expected from the hardware, and faster.

I'm specifically trying to type-alias here in a safe way. This is defining a
union that takes both a 32 or a 64 bit number and stores it at the same
location
that is 64 bits in size.

Yes, sorry, I think this is safe here, provided that T has 64 bits. (I
mis-remembered the rules surrounding unions; there are some type-punning uses of unions that are not safe.) Having said that, Sven mentioned that Clang doesn't
do this right anyway.

Interestingly, it looks like C++ is somewhat more relaxed about type punning
than C is. In particular, it suggests that reinterpret_cast can be used in some
cases that are definitely not safe in C. I think I need to spend some time
reading up on C++ aliasing rules.


https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode180
src/arm64/simulator-arm64.h:180: STATIC_ASSERT(sizeof(union SetUnion) ==
sizeof(value_));
On 2014/03/27 17:50:42, jbramley wrote:
> That assertion isn't sufficient; if T is smaller than int64_t, sizeof(reg)
will
> still match sizeof(value_).

Which is what I want. From a conceptual standpoint I think that a union fits what is going on with the register. It has 64 bits, but in 32 bit mode only
half is accessible.  But they have the same address.  Just like a union.

Sort-of, yes, but a 32-bit write to a 64-bit register does actually write 64
bits, so it's not a perfect fit.

I'll agree the initialization was incorrect. C89 only initializes the first
element.  I can't find documentation on what happens if only the smaller
element
is initialized. You are probably right that the top half is then undefined.

C99 also says that the other bits are undefined. C++ initialization is a little
more complicated, but I think it has the same effect in this case.


https://codereview.chromium.org/213943002/

--
--
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.

Reply via email to