[PATCH] D48190: [MCA][x86][NFC] Add tests for -register-file-stats, -scheduler-stats

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: test/tools/llvm-mca/X86/register-file-statistics.s:45
+# SKLSER-NEXT:    Total number of mappings created:    2
+# SKLSER-NEXT:    Max number of mappings used:         2
+
----------------
lebedev.ri wrote:
> andreadb wrote:
> > lebedev.ri wrote:
> > > RKSimon wrote:
> > > > A lot of these prefixes look redundant?
> > > Oh, indeed.
> > > 
> > > BTW, in this file, everything before `Register File statistics:` isn't expected to be here.
> > > Can
> > > ```
> > > Iterations:        100
> > > Instructions:      100
> > > Total Cycles:      51
> > > Dispatch Width:    2
> > > IPC:               1.96
> > > Block RThroughput: 0.5
> > > ```
> > > be named a "-basic-stats", and an option to control that output be added?
> > > @andreadb does that sound reasonable?
> > Not really.
> > I think the summary view should never be optional in llvm-mca.
> > It is only a few lines, and it gives a nice overview of the run.
> > 
> I agree that it is quite essential for normal usage,
> but in the edge-case use-case like this, it does not seem to be of much use..
My opinion is that register pressure should be always put into perspective. 
Register pressure tends to change over the course of a simulation for many reasons (i.e. the presence of long latency instructions, long data dependencies, lack of resources versus high dispatch rate). So, the number of iterations usually matters, and it helps putting the register usage into perspective.
That being said, if other reviewers are okay with this change, then I won't oppose it.


Repository:
  rL LLVM

https://reviews.llvm.org/D48190