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);
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.
https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode169
src/arm64/simulator-arm64.h:169: if (size == kSRegSizeInBits) {
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.
https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode179
src/arm64/simulator-arm64.h:179: } reg = { new_value };
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.
}
}
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_));
That assertion isn't sufficient; if T is smaller than int64_t,
sizeof(reg) will still match sizeof(value_).
https://codereview.chromium.org/213943002/diff/20001/src/arm64/simulator-arm64.h#newcode182
src/arm64/simulator-arm64.h:182: value_ = reg.value;
Again, if T is smaller than value_, this won't correctly zero-extend it.
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.
