This is an archive of the discontinued LLVM Phabricator instance.

[X86] Fix skylake server scheduling info.
ClosedPublic

Authored by courbet on Jun 4 2018, 8:26 AM.

Details

Summary

This fixes most of the scheduling info for SKX vector operations.
I had to split a lot of the YMM/ZMM classes into separate classes for YMM and ZMM.

I've left out the DQ/QQ issues for now. I'll fix them in a separate change.
There seems to be several cases of broadcast/permutations specialized on the mask value, I have not touched these.

The before/after llvm-exegesis analysis are in the phabricator diff.

Diff Detail

Event Timeline

courbet created this revision.Jun 4 2018, 8:26 AM
RKSimon added a subscriber: RKSimon.

Please can you pull the unsupported diff for existing classes out into a separate patch for inital review? This patch can then be just about the changes for SkylakeServer

Please can you pull the unsupported diff for existing classes out into a separate patch for inital review? This patch can then be just about the changes for SkylakeServer

Sure, will do.

courbet planned changes to this revision.Jun 11 2018, 1:39 AM
courbet edited the summary of this revision. (Show Details)Jun 11 2018, 5:00 AM
courbet updated this revision to Diff 150713.Jun 11 2018, 5:02 AM
  • Use the new WriteResUnsupported multiclasses
  • More sched fixes
courbet added a comment.EditedJun 11 2018, 5:02 AM

Before:

After:

courbet updated this revision to Diff 150715.Jun 11 2018, 5:08 AM
  • remove more comment noise
courbet updated this revision to Diff 150716.Jun 11 2018, 5:32 AM
  • split SKXWriteResGroup93.
  • split SKXWriteResGroup50

@RKSimon I think this is ready for review now.

Nice! A couple of minors - but almost there I reckon.

lib/Target/X86/X86SchedSkylakeServer.td
337

You results suggest WritePHMINPOS should be just Port0?

378–385

The QQ conversions might be best off with instrw overrides?

390

The QQ conversions might be best off with instrw overrides?

406

WriteCvtPH2PSY is the one that does pass your tests - it's WriteCvtPH2PS and WriteCvtPH2PSZ that are irregular?

609

Worth pulling the LAHF/SAHF change out into its own patch as IIRC a lot of Intel models have dodgy values for this?

courbet updated this revision to Diff 150723.Jun 11 2018, 5:47 AM
courbet marked an inline comment as done.
  • address review comments

Thanks !

lib/Target/X86/X86SchedSkylakeServer.td
378–385

I guess so. I'll do it in a separate commit as there are already a lot of changes here :(

406

Thanks for the catch. This has been addressed indeed. The other two are a tiny bit irregular but nothing crazy - I think it's just noise.

609

Will do.

courbet updated this revision to Diff 150725.Jun 11 2018, 5:49 AM
  • revert LAHF/SAHF changes
Harbormaster completed remote builds in B19161: Diff 150725.
RKSimon accepted this revision.Jun 11 2018, 6:49 AM

LGTM with a few final minor corrections.

lib/Target/X86/X86SchedSkylakeServer.td
980

Keep these aligned

1074

Merge this into "(V?)CVTSD2SS(Z?)rr" ?

1371

Don't use instregex unless you really need to - it take a lot longer than a basic instrs match. All of these look like they will have a single match?

This revision is now accepted and ready to land.Jun 11 2018, 6:49 AM
courbet marked 3 inline comments as done.Jun 11 2018, 7:13 AM

Thanks a lot for the review.

courbet updated this revision to Diff 150749.Jun 11 2018, 7:23 AM
  • update MCA test
  • update sched tests
courbet updated this revision to Diff 150751.Jun 11 2018, 7:32 AM

More test updating.

courbet updated this revision to Diff 150752.Jun 11 2018, 7:38 AM

More test updating.

This revision was automatically updated to reflect the committed changes.