[X86] Disable CLWB in Cannon Lake

[GBuella]

Summary

Cannon Lake does not support CLWB, therefore it
does not include all features listed under SKX anymore.

Instead, enumerate all SKX features with the exception of CLWB.

[echristo]

Testcase?

[craig.topper]

This bug exists in clang too.

[craig.topper]

How do we test this? You'd have to verify that the clwb intrinsic throws a cannot select error on cannonlake, but do we do that in llc lit tests today?

[echristo]

We have some testcases like this, but not a lot. We could add one - also we should probably not assemble if we're using cannonlake as well. The aarch64 backend has some examples of llvm-mc tests in this case.

In D43380#1010794, @craig.topper wrote:

How do we test this? You'd have to verify that the clwb intrinsic throws a cannot select error on cannonlake, but do we do that in llc lit tests today?

[craig.topper]

I thought our assemble stance was that everything was available all the time. For example, the bug a few weeks back that said we should always parse xmm16-xmm31 registers regardless of avx512 enabling. Or are we saying we should support everything unlesss you pick a specific CPU?

[GBuella]

In D43380#1010794, @craig.topper wrote:

How do we test this? You'd have to verify that the clwb intrinsic throws a cannot select error on cannonlake, but do we do that in llc lit tests today?

I have not seen any such tests for CPU being selected in LLVM.
The closest thing I found is in clang ( test/Preprocessor/predefined-arch-macros.c ), I'm adding it there.

[GBuella]

See also: https://reviews.llvm.org/D43459

[craig.topper]

You can do something like test/CodeGen/X86/pr33772.ll

[GBuella]

Adding tests for checking that the
cpu flags affect clwb intrinsic usage as they
should.

[GBuella]

In D43380#1010942, @craig.topper wrote:

I thought our assemble stance was that everything was available all the time. For example, the bug a few weeks back that said we should always parse xmm16-xmm31 registers regardless of avx512 enabling. Or are we saying we should support everything unlesss you pick a specific CPU?

Yes, the MC assembler allows any instruction, so this patch has no relation to that.

[craig.topper]

LGTM

lib/Target/X86/X86.td

class SkylakeServerProc<string Name> : ProcModel<Name, SkylakeServerModel,
                                                  SKXFeatures.Value, [
   ProcIntelSKX,
   FeatureHasFastGather,
   FeaturePOPCNTFalseDeps
 ]>;
 def : SkylakeServerProc<"skylake-avx512">;
 def : SkylakeServerProc<"skx">; // Legacy alias.
 
-def CNLFeatures : ProcessorFeatures<SKXFeatures.Value, [
+def CNLFeatures : ProcessorFeatures<SKLFeatures.Value, [
+  FeatureAVX512,
+  FeatureCDI,
+  FeatureDQI,
+  FeatureBWI,
+  FeatureVLX,
+  FeaturePKU,
   FeatureVBMI,
   FeatureIFMA,
   FeatureSHA
 ]>;
 
 class CannonlakeProc<string Name> : ProcModel<Name, SkylakeServerModel,
                                               CNLFeatures.Value, [
   ProcIntelCNL,

test/CodeGen/X86/clwb.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; NOTE: clwb is available in Skylake Server, not available in the newer
+; NOTE: Cannon Lake arch, but available again in the newer Ice Lake arch.
 ; RUN: llc < %s -mtriple=i686-apple-darwin -mattr=clwb | FileCheck %s
+; RUN: llc < %s -mtriple=i686-apple-darwin -mcpu=skx | FileCheck %s
+; RUN: not llc < %s -mtriple=i686-apple-darwin -mcpu=cannonlake 2>&1 | FileCheck %s --check-prefix=CNL
+; RUN: llc < %s -mtriple=i686-apple-darwin -mcpu=icelake | FileCheck %s
+
+; CNL: LLVM ERROR: Cannot select: intrinsic %llvm.x86.clwb
 
 define void @clwb(i8* %p) nounwind {
 ; CHECK-LABEL: clwb:
 ; CHECK:       ## %bb.0:
 ; CHECK-NEXT:    movl {{[0-9]+}}(%esp), %eax
 ; CHECK-NEXT:    clwb (%eax)
 ; CHECK-NEXT:    retl
   tail call void @llvm.x86.clwb(i8* %p)
   ret void
 }
 declare void @llvm.x86.clwb(i8*) nounwind