This is an archive of the discontinued LLVM Phabricator instance.

[Subtarget] Move SubtargetFeatureKV/SubtargetInfoKV from SubtargetFeature.h to MCSubtargetInfo.h. Move all code that operates on ProcFeatures and ProcDesc arrays to MCSubtargetInfo.
ClosedPublic

Authored by craig.topper on Mar 4 2019, 4:48 PM.

Details

Summary

The SubtargetFeature class managed a list of features as strings. And it also had functions for setting bits in a FeatureBitset.

The methods that operated on the Feature list as strings are used in other parts of the backend. But the parts that operate on FeatureBitset are very tightly coupled to MCSubtargetInfo and requires passing in the arrays that MCSubtargetInfo owns. And the same struct type is used for ProcFeatures and ProcDesc.

This has led to MCSubtargetInfo having 2 arrays keyed by CPU name. One containing a mapping from a CPU name to its features. And one containing a mapping from CPU name to its scheduler model.

I would like to make a single CPU array containing all CPU information and remove some unneeded fields the ProcDesc array currently has. But I don't want to make SubtargetFeatures.h have to know about the scheduler model type and have to forward declare or pull in the header file.

Diff Detail

Repository
rL LLVM

Event Timeline

craig.topper created this revision.Mar 4 2019, 4:48 PM

LGTM.

lib/MC/MCSubtargetInfo.cpp
60 ↗(On Diff #189233)

Maybe add an error message to this assert. Something like "expected a valid feature flag".

153 ↗(On Diff #189233)

Can there be repeated '+help' features?
If so, then we may potentially end up printing multiple "help" messages. That being said, it looks like this was how it worked even before your patch. So, I am okay if you don't change it.

andreadb accepted this revision.Mar 5 2019, 3:20 AM
This revision is now accepted and ready to land.Mar 5 2019, 3:20 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 5 2019, 10:53 AM