[PATCH] D137440: [RISCV][llvm-mca] Use LMUL Instruments to provide more accurate reports on RISCV

Andrea Di Biagio via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 7 15:05:25 PST 2022


andreadb added a comment.

In D137440#3913468 <https://reviews.llvm.org/D137440#3913468>, @michaelmaitland wrote:

> - Updated docs
> - Made fields in `Instrument` private
> - Improved classdoc for `InstrumentManager`
> - Removed `llvm` namespace qualifier where it wasn't needed
> - Simplified `Instrument` default constructor
> - Use `StringSwitch` in `RISCVCustomBehaviour`
> - Use `==` instead of `.equals` on StringRefs
> - add `const` qualifier on `MCACommentConsumer::hadErr`
>
> With respect to the request that tests are added:
>
> There is no RISCV vector scheduler model that exists upstream. As a result, any tests I would add
> will fail because processor does not support the vector instructions. I propose that we should create
> a generic RISCV vector processor that exists upstream in a seperate patch. I think that this
> would be beneficial for MCA so I can include those tests, but also for llvm contributors in areas
> such as academia. What do you think about this?

Sounds like a good idea. Thanks!

I don't have other comments. Patch looks good to me. @myhsu what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137440



More information about the llvm-commits mailing list