this is clang part , the llvm part is https://reviews.llvm.org/differential/diff/114515/
Details
Diff Detail
- Repository
- rL LLVM
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 ↗ | (On Diff #114765) | 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 ↗ | (On Diff #114765) | We usually don't declare variables in the intrinsics if we can avoid it. Just nest the calls. |
test/CodeGen/avx512vl-builtins.c | ||
4511 ↗ | (On Diff #114765) | The first line is over indented |
4525 ↗ | (On Diff #114765) | 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 ↗ | (On Diff #114836) | weird indentation? |
5734 ↗ | (On Diff #114836) | weird indentation? |
5741 ↗ | (On Diff #114836) | weird indentation? |
5748 ↗ | (On Diff #114836) | weird indentation? |
test/CodeGen/avx512f-builtins.c | ||
7732 ↗ | (On Diff #114836) | weird indentation? |
7755 ↗ | (On Diff #114836) | weird indentation? |
7966 ↗ | (On Diff #114836) | weird indentation? |
test/CodeGen/avx512vl-builtins.c | ||
4597 ↗ | (On Diff #114836) | weird indentation? |
include/clang/Basic/BuiltinsX86.def | ||
---|---|---|
981 ↗ | (On Diff #114765) | 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 ↗ | (On Diff #114978) | Please remove the #ifdef x86_64 from this. It should work in 32-bits as well. |
lib/Headers/avx512vlintrin.h | ||
5759 ↗ | (On Diff #114978) | Please remove the x86_64 from these. They should work in 32-bit mode. |
lib/Headers/avx512fintrin.h | ||
---|---|---|
9742 ↗ | (On Diff #114978) | the current generated code for 32-bit is not optimal, 32-bit needs some more work which will be in a following patch . |