This is an archive of the discontinued LLVM Phabricator instance.

[X86] Disable CLWB in Cannon Lake
ClosedPublic

Authored by GBuella on Feb 16 2018, 3:29 AM.

Details

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.

Diff Detail

Event Timeline

GBuella created this revision.Feb 16 2018, 3:29 AM

This bug exists in clang too.

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?

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.

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 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?

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.

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

GBuella updated this revision to Diff 135022.Feb 20 2018, 1:46 AM

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

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.

This revision is now accepted and ready to land.Feb 20 2018, 9:31 AM
This revision was automatically updated to reflect the committed changes.