AVX10 Architecture Specification: https://cdrdv2.intel.com/v1/dl/getContent/784267
AVX10 Technical Paper: https://cdrdv2.intel.com/v1/dl/getContent/784343
RFC: https://discourse.llvm.org/t/rfc-design-for-avx10-feature-support/72661
Details
- Reviewers
RKSimon craig.topper skan e-kud
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
clang/lib/Basic/Targets/X86.h | ||
---|---|---|
99 | Maybe should be HasAVX10_1_512? As brought up the rfc, there might be an avx10.2-512 Likewise elsewhere, or is this to match GCC? |
clang/lib/Basic/Targets/X86.h | ||
---|---|---|
99 | No. HasAVX10_512BIT is a single feature which can be combined with HasAVX10_1, HasAVX10_2 etc. |
Ping~ It looks to me there's no concern about this solution in the RFC. I think we can move forward to land it.
clang/lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
2581 | Minor suggestion. The code here may be refined to be better extended by other targets, like llvm::Triple::ArchType ArchType = getContext().getTargetInfo().getTriple().getArch(); switch (ArchType) { case llvm::Triple::x86: case llvm::Triple::x86_64: { .... } default: return; | |
llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp | ||
924 | I think what you need here is TSFlags & X86II::EVEX_L2 instead of getL2(). The class X86OpcodePrefixHelper is designed for encoding only. The bit L2 can be set in other cases so it may blur the meaning of 512-bit here you use the getter. | |
926 | -mavx10.1 does not work for assembler. So if such instruction is generated w/o AVX10-512BIT support, it must be compiler's issue instead of user's. An assert should be more appropriate here. |
llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp | ||
---|---|---|
926 |
Reference: https://llvm.org/docs/CodingStandards.html#assert-liberally |
Address comment.
clang/lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
2581 | We have a few place code using isX86. I think it's more convenient to use a single condition. We can refactor when necessary. | |
llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp | ||
926 | We need to report fatal error for this case even if it's a compiler bug. Otherwise, user may observe the crash issue in runtime and hard to find the reason. |
clang/lib/Basic/Targets/X86.cpp | ||
---|---|---|
739 | This is untested? |
Just curious, in RFC we have -mavx10.x-256/-mavx10.x-512 but here we refer to -mavx10.x/-mavx10.x,-mavx10-512bit. Is it compliant with GCC, or the revision is just for the illustrative purpose?
Sorry for the late reply. We have received a couple concerns about how to interpret these options, especially when used together with AVX512 options. We decided not to provide AVX10.1 options at the present, instead, we just provide -m[no-]evex512 to disable ZMM and 64-bit mask instructions for AVX512 features. For more details, lease take a look at D159250.
clang/lib/Basic/Targets/X86.cpp | ||
---|---|---|
739 | Done in an alternative reversion D159250. | |
clang/lib/Driver/ToolChains/Arch/X86.cpp | ||
261 | Thanks! This is not needed in the new version. |
Maybe should be HasAVX10_1_512? As brought up the rfc, there might be an avx10.2-512
Likewise elsewhere, or is this to match GCC?