This is an archive of the discontinued LLVM Phabricator instance.

[X86] Attempt to make the Intel core CPU inheritance a little more readable and maintainable
ClosedPublic

Authored by craig.topper on Jun 7 2019, 12:14 PM.

Details

Summary

The recently added cooperlake CPU has made our already ugly switch statement even worse. There's a CPU exclusion list around the bf16 feature in the cooper lake block. I worry that we'll have to keep adding new CPUs to that until bf16 intercepts a client space CPU. We have several other exclusion lists in other parts of the switch due to skylakeserver, cascadelake, and cooperlake not having sgx. Another for cannonlake not having clwb but having all other features from skx.

This removes all these special ifs at the cost of some duplication of features and a goto. I've copied all of the skx features into either cannonlake or icelakeclient(for clwb). And pulled sklyakeserver, cascadelake, and cooperlake out of the main inheritance chain into their own chain. At the end of skylakeserver we merge back into the main chain at skylakeclient but below sgx. I think this is at least easier to follow. Do others agree or have alternate suggestions of what to do here? Of course the best answer is probably this should all be in a table somehow.

Diff Detail

Event Timeline

craig.topper created this revision.Jun 7 2019, 12:14 PM
craig.topper edited subscribers, added: cfe-commits; removed: llvm-commits.

Fix mistake in a comment

RKSimon accepted this revision.Jun 10 2019, 2:27 AM

LGTM - the goto is unfortunate but more maintainable and less confusing then the current state of things

clang/lib/Basic/Targets/X86.cpp
157

Worth adding a comment explaining the AVX512 ISA divergence?

This revision is now accepted and ready to land.Jun 10 2019, 2:27 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2019, 9:59 AM