This is an archive of the discontinued LLVM Phabricator instance.

[X86][LLVM][TD] Arranging Atom family in an inheritance ratio
Needs ReviewPublic

Authored by m_zuckerman on Jul 13 2017, 6:48 AM.

Details

Summary

X86.td file contains number of features that can be arrange in more general pattern . We can combine features that include in number of atom into single processor and create a inheritance ratio between them all.

Diff Detail

Event Timeline

m_zuckerman created this revision.Jul 13 2017, 6:48 AM
craig.topper added inline comments.Jul 16 2017, 11:11 PM
lib/Target/X86/X86.td
428–443

This looks to be adding RDRAND to SLM. Whether that's correct or not, that's a functional change which should not be part of this patch.

437

Should SSSE3 be part of AtomValues instead? The rest of these are uarch details, but that one is a real feature and should be with the other features.

m_zuckerman added inline comments.Jul 17 2017, 5:07 AM
lib/Target/X86/X86.td
428–443

hi.
First, this feature is part of SLM.
Secondly , why don't you want it as part of the patch ?

437

Higher features hint for lower features. In this case feature SSSE3 is part of the SSE42 so i didn't want to create a duplication.

craig.topper added inline comments.Jul 17 2017, 7:03 AM
lib/Target/X86/X86.td
428–443

I agree that it's part of SLM and that it should be fixed. But this patch should be treated as a refactor that doesn't change behavior. Especially since your description makes no mention of this change. And clang needs to be updated too. So submit a separate pre or post patch for the SLM rdrand change.

I returned the RDRAND into the Goldmont and I will fix it later in another patch.

craig.topper added inline comments.Jul 26 2017, 10:49 PM
lib/Target/X86/X86.td
415

Why is CallRegIndirect in BonnellProc and AtomPlusFeatures? Should it be in AtomFeatures instead? Or should each CPU list is separately?

417

I don't think the "Slow" features should be part of AtomPlusFeatures. While its great for sharing between Silvermont and Goldmont. It makes AtomPlusFeatures hard to inherit from for the processor after Goldmont if it changes any of those things.