[PATCH] D26398: [mips][msa] Implement f16 support

[Zoran Jovanovic via llvm-commits]

zoran.jovanovic added inline comments.


================
Comment at: lib/Target/Mips/MipsMSAInstrInfo.td:3747
+
+ def MSA_FP_EXTEND_W_PSEUDO : MipsPseudo<(outs FGR32Opnd:$fd), (ins MSA128F16:$ws),
+                              [(set FGR32Opnd:$fd, (f32 (fpextend MSA128F16:$ws)))]> {
----------------
Nit: Next two lines exceed 80 characters.


================
Comment at: lib/Target/Mips/MipsMSAInstrInfo.td:3752
+
+ def MSA_FP_ROUND_W_PSEUDO : MipsPseudo<(outs MSA128F16:$wd), (ins FGR32Opnd:$fs),
+                              [(set MSA128F16:$wd, (f16 (fpround FGR32Opnd:$fs)))]> {
----------------
Nit: Next two lines exceed 80 characters.


================
Comment at: lib/Target/Mips/MipsMSAInstrInfo.td:3757
+
+ def MSA_FP_EXTEND_D_PSEUDO : MipsPseudo<(outs FGR64Opnd:$fd), (ins MSA128F16:$ws),
+                              [(set FGR64Opnd:$fd, (f64 (fpextend MSA128F16:$ws)))]> {
----------------
Nit: Next two lines exceed 80 characters.


================
Comment at: lib/Target/Mips/MipsMSAInstrInfo.td:3762
+
+ def MSA_FP_ROUND_D_PSEUDO : MipsPseudo<(outs MSA128F16:$wd), (ins FGR64Opnd:$fs),
+                              [(set MSA128F16:$wd, (f16 (fpround FGR64Opnd:$fs)))]> {
----------------
Nit: Next two lines exceed 80 characters.


================
Comment at: lib/Target/Mips/MipsSEISelLowering.cpp:3448
+
+  const bool IsN64 = Subtarget.isABI_N64();
+  const TargetRegisterClass *RC =
----------------
Nit: I do believe that Mips::GPR64RegClass and 64-bit variants of instructions should be used with N32 too.
Same is for code in MipsSETargetLowering::emitLD_F16_PSEUDO.


================
Comment at: lib/Target/Mips/MipsSEISelLowering.cpp:3655
+// =>
+//  fexupr.w $wtemp, $ws
+//  copy_s.w $rtemp, $wtemp[0]
----------------
Nit: Is this comment correct? 
>From the code below (and corresponding test cases) it seems that fexupr.d instruction should be generated after this one.



================
Comment at: lib/Target/Mips/MipsSEISelLowering.cpp:3689
+  BuildMI(*BB, MI, DL, TII->get(Mips::FEXUPR_W), Wtemp).addReg(Ws);
+  if (IsFGR64) {
+    WPHI = RegInfo.createVirtualRegister(&Mips::MSA128WRegClass);
----------------
if comment above is correct IsFGR64 should be replaced with IsFGR64onMips64.


Repository:
  rL LLVM

https://reviews.llvm.org/D26398