[PATCH] D90738: [RISCV] Support Zfh half-precision floating-point extension.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1521
 };
+static const MCPhysReg ArgFPR16s[] = {RISCV::F10_H, RISCV::F11_H, RISCV::F12_H,
+                                      RISCV::F13_H, RISCV::F14_H, RISCV::F15_H,
----------------
Can we line break this the same way as FPR32s/FPR64s for consisency?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1705
   if (ValVT == MVT::f32 && !UseGPRForF32)
     Reg = State.AllocateReg(ArgFPR32s, ArgFPR64s);
   else if (ValVT == MVT::f64 && !UseGPRForF64)
----------------
The second argument is shadow regs. Do we need a way to shaddow ArgFPR16s here.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:1709
+  else if (ValVT == MVT::f16 && !UseGPRForF16)
+    Reg = State.AllocateReg(ArgFPR16s, ArgFPR32s);
   else
----------------
Does this also need to shadow ArgFPR64s?


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:2836
         return std::make_pair(DReg, &RISCV::FPR64RegClass);
+      } else if (Subtarget.hasStdExtZfh()) {
+        unsigned HReg = RISCV::F0_H + RegNo;
----------------
It looks like this code was trying to pick the largest register size. Shouldn't we keep the _F register if StdExtD is disabled regardless of whether Zfh is enabled?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90738