[llvm] [RISCV][MC] Recognise that fcvt.d.s with frm != 0b000 is valid (PR #67555)

Alex Bradbury via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 30 06:59:11 PDT 2023


asb wrote:

> > Do we end up printing the `rne` with no-aliases now? The check lines don't check the whole line so they wouldn't fail if an extra operand started printing.
> 
> The InstAlias _should_ mean no (and is working as expected for D), but it seems it's not working as expected for dinx. Agree I need to update this with proper test coverage for that.
> 
> I wondered about the adding another FRMArg variant with a different defaultFRMArgOps but instead, but it seemed a shame to add new tablegen classes for this. Maybe it's a better solution overall though - I'll have more of a play with it.

1) Commit dc1dc60ea8786d4a2462d157842501d76593f9e7 adds test coverage for the printing of the rounding mode with/without aliases enabled.
2) I've rebased this PR on top of it (although rebasing PRs is discouraged in general, rebasing to account for a newly added test is one of the acceptable reasons in our docs)
3) I've added a commit that instead of using InstAlias, introduces new operand types and the required associated functions. It adds a bit more than I'd like, but it does seem better to not have to define the InstAlias. There are other cases to fix (fcvt.d.w, fcvt.d.wu, fcvt.s.h, fcvt.d.h) so this extra complexity will be somewhat amortized by not needing explicit InstAlias definitions for those cases too.

I think this is ready for full review - thanks!

https://github.com/llvm/llvm-project/pull/67555


More information about the llvm-commits mailing list