This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Separate Features that are known to be Power9 specific from Future CPU
ClosedPublic

Authored by stefanp on Nov 19 2019, 2:39 PM.

Details

Summary

The Power 9 CPU has some features that are unlikely to be passed on to future versions of the CPU.
This patch separates this out so that future CPU does not inherit them.

Diff Detail

Event Timeline

stefanp created this revision.Nov 19 2019, 2:39 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 19 2019, 2:39 PM

The feature list here assume that, new processor will contain superset of features of those that came before it. And we are trying to break the assumption here. Can we do it another way. That is, keep the TD unchanged and check the CPU Directive in PPCSubtarget ? i.e.

PPCSubtarget::vectorsUseTwoUnits() const { return VectorsUseTwoUnits && DarwinDirective == PPC::DIR_PWR9; }

The feature list here assume that, new processor will contain superset of features of those that came before it. And we are trying to break the assumption here. Can we do it another way. That is, keep the TD unchanged and check the CPU Directive in PPCSubtarget ? i.e.

PPCSubtarget::vectorsUseTwoUnits() const { return VectorsUseTwoUnits && DarwinDirective == PPC::DIR_PWR9; }

I strongly disagree with this suggestion. The CPU model should reflect the features that the CPU supports. Separating out checks this way is an unnecessary separation of concerns that make it difficult for the uninitiated reader to decipher what CPU has which feature.
The fact is that the Power9 CPU has some quirks that make it stand out from other CPUs such as Power8 (i.e. no fusion capability, narrower dispatch width for vectors, etc.). There is no reason to expect that these quirks will exist in any future CPUs. It is really helpful in terms of readability to see in one place that this CPU does not follow the regular progression of Power CPUs and to see exactly what it does differently.

If we want to break the assumption, please update the comment for the processor feature which is separated into two parts now, inherit and only, for all cpus. The inherit part still has the assumption, and p9 = p9 inherit + p9 only.

nemanjai accepted this revision.Nov 21 2019, 5:56 AM

LGTM with some clarification in a comment.

llvm/lib/Target/PowerPC/PPC.td
241

Fair enough. So lets add a comment along the lines of the following here:

// Some features are unique to Power9 and there is no reason to assume
// they will be part of any future CPUs. One example is the narrower
// dispatch for vector operations than scalar ones. For the time being,
// this list also includes scheduling-related features since we do not have
// enough info to create custom scheduling strategies for future CPUs.
This revision is now accepted and ready to land.Nov 21 2019, 5:56 AM
This revision was automatically updated to reflect the committed changes.