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.
https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode169
src/arm64/simulator-arm64.h:169: if (size == kSRegSizeInBits) {
On 2014/03/27 17:50:42, jbramley wrote:
Why kSRegSize here? These can be any register type (currently X, W, D,
S). I
think we would use kWRegSize by default, perhaps with a static
assertion as you
used to check that kSRegSize == kWRegSize.
I choose kSRegSize because I was following the logic in fpreg. I now
see this was incorrect.
Done.
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.
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.
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.
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.
