> The compiler does not > have to discard what it can infer about the alignment just because you > cast 'mem' to a type with weaker alignment constraints. > > Why does 'mem' need to have type 'sse_union *'? Why can't it just be > declared as 'uint8_t *'? Huh, I see. I'll just delete sse_union then and use uint32_t instead. > Ewwww. That's likely because emulator.c does: > #define memset __builtin_memset > As evidenced by this issue, using the compiler's memset() in kvm-unit-tests seems > inherently dangerous since the tests are often doing intentionally stupid things. I'll make a separate patch to remove this from emulator.c On Thu, May 6, 2021 at 1:11 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote: > > On Thu, May 6, 2021 at 12:14 PM Jacob Xu <jacobhxu@xxxxxxxxxx> wrote: > > > > > memset() takes a void *, which it casts to an char, i.e. it works on one byte at > > a time. > > Huh, TIL. Based on this I'd thought that I don't need a cast at all, > > but doing so actually results in a movaps instruction. > > I've changed the cast back to (uint8_t *). > > I'm pretty sure you're just getting lucky. If 'mem' is not 16-byte > aligned, the behavior of the code is undefined. The compiler does not > have to discard what it can infer about the alignment just because you > cast 'mem' to a type with weaker alignment constraints. > > Why does 'mem' need to have type 'sse_union *'? Why can't it just be > declared as 'uint8_t *'? Just add a "memory" clobbers to the inline > asm statements that use 'mem' as an SSE operand. > > Of course, passing it as an argument to sseeq() also implies 16-byte > alignment. Perhaps sseeq should take uint32_t pointers as arguments > rather than sse_union pointers. I'm not convinced that the sse_union > buys us anything other than trouble.
