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?