This is an archive of the discontinued LLVM Phabricator instance.

[X86] Generalize schedule classes to support multiple stages
ClosedPublic

Authored by RKSimon on Mar 18 2018, 3:17 PM.

Details

Summary

Currently the WriteResPair style muliclasses take a single pipeline stage and latency, this patch generalizes this to make it easier to use with more complex schedules with ResourceCycles and NumMicroOps to be overriden from their defaults.

This has already been done for the Jaguar scheduler to remove a number of custom schedule classes and adding it to the other x86 targets will make it much tidier as we add additional classes in the future to try and replace so many custom cases.

I've converted some instructions but a lot of the models need a bit of cleanup after the patch has been committed - memory latencies not being consistent, the class not actually being used when we could remove some/all customs, etc. I'd prefer to keep this as NFC as possible so later patches can be smaller and target specific.

Diff Detail

Repository
rL LLVM

Event Timeline

RKSimon created this revision.Mar 18 2018, 3:17 PM
RKSimon added inline comments.Mar 18 2018, 3:21 PM
test/CodeGen/X86/avx2-schedule.ll
612 ↗(On Diff #138867)

This is the only test change - Sandy Bridge doesn't have AVX2, but it does actually override the xmm variant but to the same values as WriteMPSAD used here - there is a lot of duplication like this....

courbet added inline comments.Mar 19 2018, 2:31 AM
lib/Target/X86/X86SchedBroadwell.td
83 ↗(On Diff #138867)

I think making Res explicit would be safer (see my comment on line 108).

108 ↗(On Diff #138867)

The default value on Res will create a inconsistent values ExePorts=[] ResourceCycles=[1].

RKSimon added inline comments.Mar 19 2018, 4:49 AM
lib/Target/X86/X86SchedBroadwell.td
108 ↗(On Diff #138867)

This patch only affects the classes that use BWWriteResPair (and equivalents on other targets) - WriteIMulH doesn't have a Ld equivalent so uses the 'raw' WriteRes - its not affected by this change.

courbet accepted this revision.Mar 19 2018, 5:19 AM
courbet added inline comments.
lib/Target/X86/X86SchedBroadwell.td
108 ↗(On Diff #138867)

Oops. I scanned the file quickly to show an example for my comment on line 83. Please ignore this one, but the comment still holds, there's nothing that prevents forgetting overriding this when ResourceCycles when providing two resources.
Arguably this should be a warning in tablegen, I've created PR36797 for this, so feel free to ignore these two comments.

This revision is now accepted and ready to land.Mar 19 2018, 5:19 AM
This revision was automatically updated to reflect the committed changes.