[PATCH] D40078: [x86][icelake]VAES introduction

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Support/Host.cpp:433
   FEATURE_SHA,
+  FEATURE_VAES,
 };
----------------
coby wrote:
> craig.topper wrote:
> > This isn't needed if it isn't used by code in getHostCPUName
> I see, thanks.
> I assume same logic is to be applied to entries like FEATURE_PCLMUL?
The first 31 values need to be kept in sync with compiler-rt since we have a copy and paste of some of this code in compiler-rt. For runtime CPU detection in compiled binaries. It makes diffing easier if they are synchronized. I just committed some comments that clarify that a little better. I'm hoping to move this enum to a header file that clang can use as a proxy for what's in compiler-rt as well since clang can't include any files from the compiler-rt project. Currently we just have a separate copy code in clang too.


================
Comment at: lib/Target/X86/X86.td:174
+                       "Promote selected AES instructions to AVX512/AVX registers",
+                        [FeatureAVX]>;
 def FeatureTBM     : SubtargetFeature<"tbm", "HasTBM", "true",
----------------
coby wrote:
> craig.topper wrote:
> > Should this imply FeatureAES as well?
> not sure,
> it has no direct dependency afaiu, like AVX, for example, i.e. it does not imply that AES insns are apparent.
> practically, if you want to use 'vaesenc', for example, you ought to have a Key, so you'll probably want to use 'aeskeygenassist' which is on AES solely, but than again - it (vaes) may still be provided on its own, so I find it yet questionable
> 
> 
The EVEX->VEX pass is going to turn 128-bit EVEX encoded AES instructions into VEX encoded versions. So those instructions better be available with VAES.


Repository:
  rL LLVM

https://reviews.llvm.org/D40078