[PATCH] D52779: AMD BdVer2 (Piledriver) Initial Scheduler model

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: lib/Target/X86/X86.td:1025
 // Piledriver
-def : Proc<"bdver2", [
+def : ProcessorModel<"bdver2", BdVer2Model, [
   FeatureX87,
----------------
courbet wrote:
> RKSimon wrote:
> > lebedev.ri wrote:
> > > craig.topper wrote:
> > > > Should we apply this to bdver3/4 as well? Are they similar?
> > > Hmm. It would be ok to apply this to bdver1.
> > > 
> > > But i'm not sure about steamroller/excavator.
> > > Those are more different, have loop buffer, 3 FPU pipes instead of 4, etc.
> > I'd expect it to help all bdver targets - but as @lebedev.ri said the arch is slightly different on bdver3/bdver4 - for instance the exegesis pfms for the pipe resources won't work correctly (and may crash).
> > 
> > We have the same problem with the generic x86_64 cpu which expects SNB pfms - @courbet how tricky would it be to attach the pfm counters to a set of CPUs instead of the scheduler model? This might even help creation of new models if we can get pfm (cycles/uops) data from cpus without a model.
> > how tricky would it be to attach the pfm counters to a set of CPUs instead of the scheduler model
> 
> The main reason why I added pfm counters to the sched model is that we want analysis to be able to match counters to resource names.
> e.g. `PfmIssueCounter<SBPort0, ["uops_dispatched_port:port_0"]>;` binds the counter to SBPort0, and the TD resolver automatically checks that SBPort0 exists in the correct sched model.
> 
> What we could do is: 
> 
>   - move the `PfmCountersInfo` out of `MCSubtargetInfo`->`MCSchedModel`->`MCExtraProcessorInfo` and put it in `MCSubtargetInfo` directly.
>   - make `SchedModel` optional and take the resource  as a string in `PfmIssueCounter` ( `PfmIssueCounter<"SBPort0", ["uops_dispatched_port:port_0"]>;`), and do the resolving/checking in the SubtargetEmitter **only if** the subtarget has a sched model.
> 
> Sounds reasonable ?
> 
> 
Yes, creating a mapping index from cpu to a pfm table list should work - it would allow cpus to share models and have their own counters to map to the models resources (some may be partial) - PR37068 could probably be dealt with at the same time.


Repository:
  rL LLVM

https://reviews.llvm.org/D52779