[PATCH] D43951: [RFC] llvm-mca: a static performance analysis tool.

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added a comment.

In https://reviews.llvm.org/D43951#1026783, @MatzeB wrote:

> This is great! I always wanted to see a IACTA like tool for other architectures.


Thanks Matthias!

> I only glanced over the patch, so this is only the nipicky/obvious things:
> 
> - New llvm code should use `camelCase` rather than `CamelCase` for method names.

Will do.

> - Too much use of `auto` for my taste. I would only use it in situations where they type is immediately obvious (on the same line) such as `auto x = dyn_cast<YYY>()` but nowhere else. I find it makes code harder to read when I first have to search around what things a `for` loop is iterating over for example.

Will do.

> - Needs documentation in `docs/CommandGuide`. Maybe you can incorporate some passages from your llvm-dev mail.

I will add a document there. I also plan to add a README.txt with the long description from the RFC.

I also plan to add more tests and restructure X86 so that there is a sub-directory for specific processors.
Something like "X86/BtVer2/dot-product.s"; etc.

Code comments can probably be improved, and I plan to do integrate a few more comments in the code.

> I would love to see this tool land in tree. And as far as I am concerned we can push it and continue development in-tree when 1) waited some more for other people to chime in here, 2) we verified the normal code isn't changed too much (double check code size of MC Unit name change) 3) we have some documentation.

I was planning to wait a few more days to give other people an opportunity to reply.

In the meantime, I am going to update a new version of the patch that addresses your review comments.

Thanks again Matthias,
-Andrea


https://reviews.llvm.org/D43951