This is an archive of the discontinued LLVM Phabricator instance.

[X86] Move CPUKind enum from clang to llvm/lib/Support. NFCI
ClosedPublic

Authored by craig.topper on Jun 8 2020, 4:42 PM.

Details

Summary

Similar to what some other targets have done. This information
could be reused by other frontends so doesn't make sense to live
in clang.

-Rename CK_Generic to CK_None to better reflect its illegalness.
-Move function for translating from string to enum into llvm.
-Call checkCPUKind directly from the string to enum translation
and update CPU kind to CK_None accordinly. Caller will use CK_None
as sentinel for bad CPU.

I'm planning to move all the CPU to feature mapping out next. As
part of that I want to devise a better way to express CPUs inheriting
features from an earlier CPU. Allowing this to be expressed in a
less rigid way than just falling through a switch. Or using gotos
as we've had to do lately.

Diff Detail

Event Timeline

craig.topper created this revision.Jun 8 2020, 4:42 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptJun 8 2020, 4:42 PM
pengfei added a subscriber: pengfei.Jun 8 2020, 5:23 PM
erichkeane added inline comments.Jun 9 2020, 6:01 AM
clang/lib/Basic/Targets/X86.h
132

This name Generic has nothing to do with the CPU-Specific 'generic' spelling, right?

llvm/include/llvm/Support/X86TargetParser.h
30

Thats an odd default... Does that mean "include 32 bit", or "just 32 bit"? And do those need to be differentiated?

craig.topper marked 2 inline comments as done.Jun 9 2020, 9:46 AM
craig.topper added inline comments.
clang/lib/Basic/Targets/X86.h
132

The backend has a "generic" but the frontend doesn't accept it. That's another reason I renamed the enum name.

llvm/include/llvm/Support/X86TargetParser.h
30

It means include all CPUs. false removes the CPUs that don't support 64 bit. A couple of the callers of getCPUKind didn't call the checkCPUKind so the default matches the behavior of not checking. For the other places I just turned the getArch() == Triple::x86 directly into this flag.

Currently we only need to remove CPUs that aren't capable of 64 bit.

Maybe I should reverse the polarity to Require64Bit or something?

erichkeane added inline comments.Jun 9 2020, 10:00 AM
clang/lib/Basic/Targets/X86.h
132

Macro brilliant:

llvm/include/llvm/Support/X86TargetParser.h
30

Something like "Include64bit"?

Flip polarity of the bool and rename it. Add doxygen comments.

erichkeane accepted this revision.Jun 9 2020, 11:37 AM
This revision is now accepted and ready to land.Jun 9 2020, 11:37 AM
This revision was automatically updated to reflect the committed changes.
skan added a subscriber: skan.Jun 10 2020, 11:37 PM