[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

Alex Bradbury via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 24 07:01:42 PST 2022


asb added a comment.
Herald added a subscriber: pcwang-thead.

Thanks for your work on this. The way you've managed to use multiclasses to handle this with the 'ExtInfo' definitions takes a bit of unpicking to follow, but it does a really good job of keeping the instruction definitions largely unmodified. Nice!

I've got a few requests/questions on the test coverage:

- I'm not sure the inclusion of the load/store instructions in the z[d,f,h]inx test cases has much value, given those instructions are unmodified?
- Given that the set of F/D/Zfh instructions that _aren't_ supported under z[d,f,h]inx is relatively short, it's probably worth being exhaustive in the *-invalid.s test cases. It's only 8 instructions for Zfinx for instance.
- The *-invalid.s test cases should have coverage for using a floating point register with an operation that is supported by Z[d,f,h]inx
- The zdinx test cases don't include any coverage for when odd registers are used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298



More information about the cfe-commits mailing list