This is an archive of the discontinued LLVM Phabricator instance.

[compiler-rt][X86] Match the detection of cpu's for __cpu_model to the latest version of gcc
ClosedPublic

Authored by craig.topper on Jul 10 2017, 11:03 AM.

Details

Summary

We were missing many feature flags that newer gcc supports and we had our own set of feature flags that gcc didnt' support that were overlapping. Clang's implementation assumes gcc's features list so a mismatch here is problematic.

I've also matched the cpu type/subtype lists with gcc and removed all the cpus that gcc doesn't support. I've also removed the fallback autodetection logic that was taken from Host.cpp. It was the main reason we had extra feature flags relative to gcc. I don't think gcc does this in libgcc.

Unfortunately, libgcc seems to have ignored their own source code comment and has added new cpu types/subtypes in the middle of their enums at various times so I can't guarantee any compatibility with older libgcc here. But maybe once we have a matching implementation we can treat that as a reason to file bugs against libgcc if they do it again going forward.

Once this support is in place we can consider implementing __builtin_cpu_is in clang. This could also be needed for function dispatching that Erich Keane is working on.

Diff Detail

Event Timeline

craig.topper created this revision.Jul 10 2017, 11:03 AM

Add back INTEL_CORE2 to the switch. Guess I got carried away with deleting.

asbirlea edited edge metadata.Jul 10 2017, 11:30 AM

Does it make sense to update this together with Host.cpp?

I can't remove the older processors from Host.cpp. Host.cpp also contains a newer processor called Goldmont that's not in libgcc yet. I think these files have slightly different goals.

echristo edited edge metadata.Jul 10 2017, 1:12 PM

I can't remove the older processors from Host.cpp. Host.cpp also contains a newer processor called Goldmont that's not in libgcc yet. I think these files have slightly different goals.

Gah.

They're not supposed to fwiw.

I'll trust you here, but can you make sure that the code in Host.cpp is as up to date with all of this as you can make it? If nothing else Host.cpp should be a strict superset of this.

-eric

I've synced what I can of this into Host.cpp now. Host.cpp now has two feature variables now because just in matching libgcc we used 31 bits. The fall back CPU detection for Intel family 6 required 3 more bits. So I gave Host.cpp a second variable. Now getAvailableFeatures in Host.cpp returns both feature variables through outparams. I've changed getAvailableFeatures here to return its single feature variable as an outparam as well to keep them somewhat consistent.

Found a mistake in my previous patch around getAvailableFeatures where I forgot to still update the Features variable that gets passed to the AMD and Intel CPU detection

asbirlea accepted this revision.Jul 12 2017, 9:29 AM

LGTM.
Thank you for the updates in Host.cpp too; the comments there are great for pointing future changes to libgcc/compiler-rt.

This revision is now accepted and ready to land.Jul 12 2017, 9:29 AM
This revision was automatically updated to reflect the committed changes.