[PATCH] D98002: [RISCV] Add scheduling resources for V

Mon Aug 2 07:26:01 PDT 2021

HsiangKai added inline comments.

================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:1178
 let Constraints = "@earlyclobber $vd" in {
-defm VFNCVT_XU_F_W : VALU_FV_VS2<"vfncvt.xu.f.w", 0b010010, 0b10000>;
-defm VFNCVT_X_F_W : VALU_FV_VS2<"vfncvt.x.f.w", 0b010010, 0b10001>;
-defm VFNCVT_RTZ_XU_F_W : VALU_FV_VS2<"vfncvt.rtz.xu.f.w", 0b010010, 0b10110>;
-defm VFNCVT_RTZ_X_F_W : VALU_FV_VS2<"vfncvt.rtz.x.f.w", 0b010010, 0b10111>;
-defm VFNCVT_F_XU_W : VALU_FV_VS2<"vfncvt.f.xu.w", 0b010010, 0b10010>;
-defm VFNCVT_F_X_W : VALU_FV_VS2<"vfncvt.f.x.w", 0b010010, 0b10011>;
-defm VFNCVT_F_F_W : VALU_FV_VS2<"vfncvt.f.f.w", 0b010010, 0b10100>;
-defm VFNCVT_ROD_F_F_W : VALU_FV_VS2<"vfncvt.rod.f.f.w", 0b010010, 0b10101>;
+defm VFNCVT_XU_F_W : VNCVTI_FV_VS2<"vfncvt.xu.f.w", 0b010010, 0b10000>;
+defm VFNCVT_X_F_W : VNCVTI_FV_VS2<"vfncvt.x.f.w", 0b010010, 0b10001>;
----------------
craig.topper wrote:
> HsiangKai wrote:
> > evandro wrote:
> > > frasercrmck wrote:
> > > > It might not really be important but it's not immediately obvious to me what some of these abbreviations signify. `VNCVTF` narrow-converts int to float but `VNCVTD` narrow-converts float to float? Why F and D?
> > > Narrowing conversion from `double` to `float` or widening from `float` to `double`.
> > ```
> > vfncvt.f.x.w  vd, vs2, vm       # Convert double-width signed integer to float.
> > vfncvt.f.f.w vd, vs2, vm        # Convert double-width float to single-width float.
> > ```
> > These two are both to convert to single-width float. Maybe use `DI`, `IF` and `DF` to distinguish them? 
> vncvt.f.f.w is also used for float to half. So using something that stands for "double" and "float" could be misleading.
I agree. It is misleading. How about use `F_X` and `F_F`  just like the instruction name.
Besides the naming issue, the patch looks good to me.

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

https://reviews.llvm.org/D98002