Conversation
| switch (type) { | ||
| case Type::i32: return Literal(i32 ^ 0x80000000); | ||
| case Type::i64: return Literal(int64_t(i64 ^ 0x8000000000000000ULL)); | ||
| case Type::i32: return Literal(-i32); |
There was a problem hiding this comment.
Mentioned previously, but both of these are potentially undefined behavior. It would be better to switch Literal to storing uint32_t and uint64_t to avoid this.
There was a problem hiding this comment.
Thanks, good catch! Fixed.
Was there a previous discussion of this? In general though we need to cast to signed/unsigned in almost all the operations in this file (e.g. for the signed/unsigned divisions), so not sure it's worth flipping the storage to unsigned or not.
There was a problem hiding this comment.
Was there a previous discussion of this?
I thought so a while back, but I could be wrong. I just know that arbitrary operations on signed values get my UB-senses tingling. Using unsigned integers is always wrapping, so it's harder to make a mistake. But if you don't expose the representation publicly then it is probably ok.
|
Oops, fuzzer found I got a float optimization here wrong. It's not ok to change |
Stuff like
x + 5 != 2=>x != -3.Also some cleanups of utility functions I noticed while writing this,
isTypeFloat => isFloatType.Inspired by
https://github.com/golang/go/blob/master/src/cmd/compile/internal/ssa/gen/generic.rules