[PATCH] D52932: [MCSched] Bind PFM Counters to the CPUs instead of the SchedModel.

[Clement Courbet via Phabricator via llvm-commits]

courbet added a comment.

In https://reviews.llvm.org/D52932#1270712, @RKSimon wrote:

> > I don't think an `MCExtraProcessorInfo` should store information for several CPUs, because `MCExtraProcessorInfo` is in `MCSchedModel`, which is selected by CPU, so that would be weird.
>
> You say that but we have multiple, very different, CPUs referencing the same model - Knights Landing uses the Haswell Model, the PileDriver model is likely to be used for the entire Bulldozer range, Sandy Bridge model is used as the default cpu - if you run llvm-exegesis in any of these cases apart for the 'main' cpu case it will just crash.


That's exactly my point: Because we have very different CPUs referencing the same SchedModel, we should not store stuff that's going to vary from CPU to CPU (e.g. `PfmCounters`) inside the `MCSchedModel`.

> 
> 
>> Before this change, we were putting the `MCPfmCountersInfo` inside `MCSchedModel`, which implicitly meant that there was a 1:1 mapping between them. This change decouples `MCPfmCountersInfo` from `MCSchedModel`.
>>  A CPU chooses its SchedModel and PfmCounters independently.
>> 
>> Now the decision we have to make now is whether in TD files (see point (2) in my comment above):
>> 
>> - (A) the CPU should declare its PfmCounters (this is the approach I'm taking here). Because that's very similar to what is done for `MCSchedModel`s, I'm doing the same for  PfmCounters, i.e. putting the table of Cpu->PfmCounter inside `TargetSubtargetInfo`, but that is actually independent from (see point (1) in my  response to Andrea's comment above)
>> - (B) the mapping of CPU->PfmCounter is kept separately (in `X86PfmCounters.td`).
> 
> (C) As the PFMs are only used by llvm-exegesis (and are likely to only every be) the PFM mappings should be moved entirely into llvm-exegesis code.

This is choice (B) here, plus choice (1) in the comment I mentioned above, quoting myself:

"That being said, I'm not opposed to moving the table of CPU->MCPfmCountersInfo outside of the MCSubtargetInfo. One possible approach is to create a new PfmEmitter tablegen backend and PfmCounters library. If people think that's reasonable I can do that easily."

So it seems we have a plan now :)

>>> One of the aims for PR39165 was to make it possible for llvm-exegesis to be run on CPUs with declared PFMs but without a model - allowing a report on raw resource usage for an instruction and to help create the model from scratch. @courbet @gchatelet do you think this would still be useful? 
>>>  Is it still an aim of llvm-exegesis to create models from scratch or just report on existing models?
>> 
>> @lebedev.ri used llvm-exegesis to produce the bdverX model. At that point there was no SchedModel, so it still seems desirable to do this.
> 
> IIRC @lebedev.ri had to create a copy of the btver2 model and then slowly iterate on it until it matched the bdver arch.




Repository:
  rL LLVM

https://reviews.llvm.org/D52932