[PATCH] D67423: [RISCV] Rename FPRs and use Register arithmetic

Tue Sep 17 05:37:07 PDT 2019

asb added a comment.

In D67423#1671595 <https://reviews.llvm.org/D67423#1671595>, @jrtc27 wrote:

> I'm still personally not fond of the `Fx_F`/`Fx_D` convention. The fact that `F0_F` < `F1_F` < `F0_D` < `F1_D` is highly surprising, especially when `F0_32` < `F0_64` < `F1_32` < `F1_64`, and not what one would naively assume. I also don't know whether this peculiarity of TableGen is meant to be something that's relied upon. I know it doesn't match the ISA convention of always calling them `Fx`, but `Fx` and `Dx` have precedence in other backends and do not risk this same confusion. Moreover, the spec itself is highly inconsistent in how to refer to single and double precision floating point:

I think with this patch, it's actually the case that F0_D is less than F0_F. I don't think it makes sense to worry about the relative ordering of register classes, just the ordering within a class. It's a shame that RISCVAsmParser has to care, but I think the static_assert is a reasonable solution there. If TableGen were to start doing something less predictable in terms of relative ordering of the register classes, we could always make a more generic function that can handle more conversions than is strictly necessary.

> - The load/store instructions are suffixed with W/D/Q
> - Other floating point instructions are suffixed with .S/.D/.Q
> - The extensions are F/D/Q
> 
>   So, personally, I would still vote for `Fx`/`Dx`/`Qx` given it avoids confusion, matches a bunch of other backends, and matches the ISA extension letters.

That's a good point, but it seems a shame to lose out on the one thing that the spec is consistent about - the architectural register name. I suppose F0 and F0_D is another alternative.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67423