[PATCH] D157485: [X86][RFC] Support new feature AVX10

[Kan Shengchen via Phabricator via cfe-commits]

skan added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:2581
+                                             unsigned VectorWidth) {
+  if (!getTarget().getTriple().isX86() || VectorWidth < 512)
+    return;
----------------
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;

```


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:925
   Prefix.setL2(TSFlags & X86II::EVEX_L2);
+  if (Prefix.getL2() && STI.hasFeature(X86::FeatureAVX10_1) &&
+      !STI.hasFeature(X86::FeatureAVX10_512bit))
----------------
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.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:927
+      !STI.hasFeature(X86::FeatureAVX10_512bit))
+    report_fatal_error("ZMM registers are not supported without AVX10-512BIT");
   switch (TSFlags & X86II::OpPrefixMask) {
----------------
-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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157485