[PATCH] D86177: [llvm-mca][NFC] Separate calculation of display data from its display in the summary and instruction info views

Wed Aug 19 13:59:41 PDT 2020

wolfgangp added a comment.

In D86177#2225748 <https://reviews.llvm.org/D86177#2225748>, @andreadb wrote:

> Nice!

Thank you Andrea!

> I agree with Roman in that we should not guarantee stability of data and/or structure.
> Also, changes to the output structure should always be advertised (for example, by adding a line in the release notes), so that people are always aware of it.

I must confess I am not very informed about the issues regarding stability. Doesn't data stability depend on the scheduling model? I don't see how llvm-mca could guarantee it if that changes. 
Or maybe I'm just confused about this. If you or Roman could briefly outline what's implied by data stability I'd appreciate it. Regarding structure stability a migration strategy would imply that llvm-mca would have to be able to read an older version JSON and update it to a later version, no?

I'll submit the follow-on patch to get started and you can educate me further on stability.

> That being said, it is not impossible for most (if not all) default views to guarantee stability of data (but not structure). After all, default views rarely (if ever) change in practice.
> However, to guarantee data stability, we need some form of versioning and ideally an "auto-upgrade" functionality too (to convert/map data from an older structure to elements of the new structure). The way how I see it is that there is no need to implement this now. We can always add more guarantees in the future if we really think it is worth it.

<snip>

- wolfgang

================
Comment at: llvm/tools/llvm-mca/Views/InstructionInfoView.cpp:29
+
+  IIVDVec IIVD(Source.size(), {0, 0, 0.0, false, false, false});
+  collectData(IIVD);
----------------
lebedev.ri wrote:
> lebedev.ri wrote:
> > These should be default member-initializers
> This is the only place with `IIVDVec`, let's just inline it?
I'm planning one more use. Would you still prefer using the explicit type?

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86177/new/

https://reviews.llvm.org/D86177