This is an archive of the discontinued LLVM Phabricator instance.

AMD znver1 Initial Scheduler model
ClosedPublic

Authored by GGanesh on Jul 12 2017, 12:24 AM.

Details

Summary

This patch adds the following

  1. Adds a skeleton scheduler model for AMD Znver1.
  2. Introduces the znver1 execution units and pipes.
  3. Caters the instructions based on the generic scheduler classes.
  4. Further additions to the scheduler model with instruction itineraries will be carried out incrementally based on a. Instructions types b. Registers used
  5. Since itineraries are not added based on instructions, throughput information are bound to change when incremental changes are added.
  6. Scheduler testcases are modified accordingly to suit the new model.

Diff Detail

Repository
rL LLVM

Event Timeline

GGanesh created this revision.Jul 12 2017, 12:24 AM
craig.topper added inline comments.Jul 12 2017, 12:31 AM
lib/Target/X86/X86Subtarget.h
61

Is the AMDZnver1 being used for anything or is this for a future patch?

GGanesh added inline comments.Jul 12 2017, 12:36 AM
lib/Target/X86/X86Subtarget.h
61

Yes, It is meant for future patch. Its a place holder for costing and alignment changes.

javed.absar added inline comments.
lib/Target/X86/X86ScheduleZnver1.td
132

As 'Latency = 1;' is default, it can be skipped just like in line 132.

RKSimon added inline comments.Jul 12 2017, 2:02 AM
lib/Target/X86/X86.td
815–816

This TODO comment can be dropped now

lib/Target/X86/X86Subtarget.h
61

It should probably be dropped from this patch and included in the future one.

GGanesh updated this revision to Diff 106835.Jul 16 2017, 10:56 PM

Updated as per the review comments.

javed.absar added inline comments.Jul 17 2017, 1:32 AM
lib/Target/X86/X86ScheduleZnver1.td
156

This would be default anyways (i.e. " let ResourceCycles = [1,1]"), so could be skipped.

224

One could make the following more concise as -
let Latency = 100 in {

def : WriteRes<WriteCLMulLd>, []>;
def : ....
def : ...

}

GGanesh updated this revision to Diff 106845.Jul 17 2017, 3:51 AM

Updated as per Javed's review comments!

RKSimon edited edge metadata.Jul 17 2017, 4:08 AM

FYI - I've been extending the scheduler tests over the weekend in trunk, you may find that you have some new failures (e.g. f16c-schedule.ll)

GGanesh updated this revision to Diff 107050.Jul 18 2017, 4:02 AM

Patch update: For newer testcases.

Simon! If you are fine, can you please commit the patch on my behalf. I am yet to get commit access rights. Probably, after this patch, I will try to get it.

Does anyone have any further comments? Otherwise I'll accept it and commit on behalf of @GGanesh

craig.topper accepted this revision.Jul 18 2017, 7:45 PM
This revision is now accepted and ready to land.Jul 18 2017, 7:45 PM
This revision was automatically updated to reflect the committed changes.

Thanks all!