[PATCH] D88396: [X86] Replace movaps with movups when avx is enabled.

Roman Lebedev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 30 01:29:23 PDT 2020


lebedev.ri added a comment.

In D88396#2302582 <https://reviews.llvm.org/D88396#2302582>, @craig.topper wrote:

> In D88396#2302542 <https://reviews.llvm.org/D88396#2302542>, @LuoYuanke wrote:
>
>> I didn't get the error at https://godbolt.org/z/8aGhd5. Another example may like this, an float array is packed in a struct.
>>
>>   #include <immintrin.h>
>>   
>>   __m128 value;
>>   
>>   typedef struct _data_str {
>>       int header;
>>       float src[400];
>>   } data_t;
>>   
>>   data_t data;
>>   
>>   void add(__m128* pointer) {
>>       value = _mm_add_ps(value, *pointer);
>>   }
>>   
>>   void foo() {
>>       for (int i = 0; i < 400; i += 4)
>>       add((__m128*)(&data.src[i]));
>>   }
>
> Compiling for SSE this code will likely use the memory form of addps which will fault on the misalignment. I know this patch only targets AVX.
>
> I don’t think you can motivate this change by showing what code you want to accept if the code would crash when compiled with the default SSE2 target.

Note that even if x86 codegen will always emit unaligned ops (which will cause new questions/bugreports),
the original IR will still contain UB, and it will be only a question of time until that causes some other 'miscompile'.
I really think this should be approached from front-end diag side.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88396/new/

https://reviews.llvm.org/D88396



More information about the llvm-commits mailing list