this is clang part , the llvm part is https://reviews.llvm.org/differential/diff/114515/
Details
Diff Detail
Event Timeline
delete from include/clang/Basic/BuiltinsX86.def and include/clang/Basic/BuiltinsX86_64.def
I think when you uploaded the changes to remove it from BuiltinsX86.def you lost your earlier changes to the header files
include/clang/Basic/BuiltinsX86.def | ||
---|---|---|
981 | I think you patch removed the only use of __builtin_ia32_pbroadcastq512_mem_mask right? Does your change work properly in 32-bit mode? | |
lib/Headers/avx512bwintrin.h | ||
2031–2033 | We usually don't declare variables in the intrinsics if we can avoid it. Just nest the calls. | |
test/CodeGen/avx512vl-builtins.c | ||
4511–4519 | The first line is over indented | |
4525–4533 | The first line is overindented |
some very minor whitespace/indentation issues
please can you confirm @craig.topper's query about __builtin_ia32_pbroadcastq512_mem_mask
lib/Headers/avx512vlintrin.h | ||
---|---|---|
5727 | weird indentation? | |
5734 | weird indentation? | |
5741 | weird indentation? | |
5748 | weird indentation? | |
test/CodeGen/avx512f-builtins.c | ||
7732 | weird indentation? | |
7755 | weird indentation? | |
7966 | weird indentation? | |
test/CodeGen/avx512vl-builtins.c | ||
4597 | weird indentation? |
include/clang/Basic/BuiltinsX86.def | ||
---|---|---|
981 | yes for both of the questions, i have deleted __builtin_ia32_pbroadcastq512_mem_mask you can see it in the new update. |
I'm going to go ahead and remove __builtin_ia32_pbroadcastq512_mem_mask from clang and change _mm512_maskz_set1_epi64 to be disabled in 32-bit mode. I want to nominate this for 5.0.1 because using it in 32-bit mode causes the compile to throw a cannot select error. So disabling it in the header at least gives a better user experience.
After that goes in you should rebase this patch and enable all of the set1_epi64 intrinsics to work in 32-bit mode like they should.
lib/Headers/avx512fintrin.h | ||
---|---|---|
9742 | Please remove the #ifdef x86_64 from this. It should work in 32-bits as well. | |
lib/Headers/avx512vlintrin.h | ||
5755 | Please remove the x86_64 from these. They should work in 32-bit mode. |
lib/Headers/avx512fintrin.h | ||
---|---|---|
9742 | the current generated code for 32-bit is not optimal, 32-bit needs some more work which will be in a following patch . |
I think you patch removed the only use of __builtin_ia32_pbroadcastq512_mem_mask right? Does your change work properly in 32-bit mode?