[PATCH] D122918: [RISCV][CodeGen] Support Zfinx, Zdinx, Zhinx, Zhinxmin codegen

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added a comment.
This revision now requires changes to proceed.

A couple of very high level review comments.

1. This patch commingles the assembler/disassembly support and the pattern matching required for IR lowering.  The typical process for an extension is to start with a patch specific to the MC layer (i.e. assembler/disassembler), and then move to codegen support in a later patch.  I strongly strongly recommend you split if you want meaningful chance of review.

2. You don't appear to have any assembler tests.  That's a show stopper right there.

3. On the MC part, please try to get the new extension as separate as possible from the existing floating point ones.  I realize there's some potential for code savings, but the change will be lower risk if it touches as little as possible.  Once it's in, and tested, changes to share code incrementally would be reasonable.

I strongly recommend you address these points.  This patch is unlikely to get meaningful review without them addressed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122918