[RISCV]A bug when llc -O0 vfmv.f.s.ll

[StephenFan]

Summary

The SUBREG_TO_REG opcode can't be eliminated when llc's optimize level is -O0. For the code show in below:
```
define dso_local double @test(<vscale x 1 x double> %v) #0 {
entry:
  %0 = call double @llvm.riscv.vfmv.f.s.f64.nxv1f64(<vscale x 1 x double> %v)
  ret double %0
}
```

when I use llc -O0 to codegen this .ll file. The mir after PostRAPseudos pass is

body:             |
  bb.0.entry:
    liveins: $v8, $x1
  
    $x2 = frame-setup ADDI $x2, -16
    CFI_INSTRUCTION def_cfa_offset 16
    SD killed $x1, $x2, 8 :: (store 8 into %stack.0)
    SD killed $x8, $x2, 0 :: (store 8 into %stack.1)
    CFI_INSTRUCTION offset $x1, -8
    CFI_INSTRUCTION offset $x8, -16
    $x8 = frame-setup ADDI $x2, 16
    CFI_INSTRUCTION def_cfa $x8, 0
    dead $x0 = PseudoVSETVLI killed $x0, 88, implicit-def $vl, implicit-def $vtype
    renamable $f0_f = PseudoVFMV_F_S_M1 killed renamable $v8, -1, implicit $vl, implicit $vtype
    $f10_f = FSGNJ_S killed $f0_f, killed $f0_f, implicit-def $f10_d
    $x8 = LD $x2, 0 :: (load 8 from %stack.1)
    $x1 = LD $x2, 8 :: (load 8 from %stack.0)
    $x2 = frame-destroy ADDI $x2, 16
    PseudoRET implicit killed $f10_d

...

The PostRVPseudos pass will lower the SUBREG_TO_REG to a instruciton of DstSubReg = COPY InsReg. So the
FSGNJ_S instruction appear. But the FSGNJ_S is the f32 register move instruction. Which is not correct for
f64 move.

[jrtc27]

The commit message could do with improving in many respects (all of spelling, grammar, formatting, style and content). Please clearly state what the actual issue is and why this change is correct given the comment that is already there to explain the current behaviour.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

3562

So is this comment just wrong? If so update it, if not your problem lies elsewhere.

[HsiangKai]

In D95234, I defined different pseudo instructions for different floating-point register classes. The floating-point vector pseudo instructions have correct register class information in D95234. I think it also solves the issue you encountered.

[StephenFan]

In D95214#2515428, @jrtc27 wrote:

The commit message could do with improving in many respects (all of spelling, grammar, formatting, style and content). Please clearly state what the actual issue is and why this change is correct given the comment that is already there to explain the current behaviour.

Thanks for your suggestion. I come from a non-English speaking country. I will try to improve my English and commit message.

[StephenFan]

In D95214#2515707, @HsiangKai wrote:

In D95234, I defined different pseudo instructions for different floating-point register classes. The floating-point vector pseudo instructions have correct register class information in D95234. I think it also solves the issue you encountered.

Ok, fine.

[craig.topper]

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

[StephenFan]

In D95214#2517277, @craig.topper wrote:

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

I have a question that why not make the scalar float point as FPR64 (not FPR32) then when encounter the FPR32 or FPR16, use the EXTRACT_SUBREG opcode.

[craig.topper]

In D95214#2517296, @StephenFan wrote:

In D95214#2517277, @craig.topper wrote:

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

I have a question that why not make the scalar float point as FPR64 (not FPR32) then when encounter the FPR32 or FPR16, use the EXTRACT_SUBREG opcode.

If you look at the current output for the fpr-spill-scalar.ll test added in D95234, you'll see that we would generate a 8 byte spill slot for float and half if we use FPR64. But the instruction to store an 8 byte F register isn't supported without the D extension.

In D95214#2517296, @StephenFan wrote:

In D95214#2517277, @craig.topper wrote:

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

I have a question that why not make the scalar float point as FPR64 (not FPR32) then when encounter the FPR32 or FPR16, use the EXTRACT_SUBREG opcode.

[StephenFan]

In D95214#2517303, @craig.topper wrote:

In D95214#2517296, @StephenFan wrote:

In D95214#2517277, @craig.topper wrote:

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

I have a question that why not make the scalar float point as FPR64 (not FPR32) then when encounter the FPR32 or FPR16, use the EXTRACT_SUBREG opcode.

If you look at the current output for the fpr-spill-scalar.ll test added in D95234, you'll see that we would generate a 8 byte spill slot for float and half if we use FPR64. But the instruction to store an 8 byte F register isn't supported without the D extension.

In D95214#2517296, @StephenFan wrote:

In D95214#2517277, @craig.topper wrote:

@StephenFan thank you for the bug report. I had a feeling that converting FPR32 to FPR64 with a SUBREG_TO_REG could cause a problem.

I have a question that why not make the scalar float point as FPR64 (not FPR32) then when encounter the FPR32 or FPR16, use the EXTRACT_SUBREG opcode.

Get it.

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasStdExtV, HasStdExtF] in {
 let mayLoad = 0, mayStore = 0, hasSideEffects = 0, usesCustomInserter = 1,
     Uses = [VL, VTYPE] in {
   foreach m = MxList.m in {
     let VLMul = m.value in {
       let HasSEWOp = 1, BaseInstr = VFMV_F_S in
-      def PseudoVFMV_F_S # "_" # m.MX : Pseudo<(outs FPR32:$rd),
+      def PseudoVFMV_F_S # "_" # m.MX : Pseudo<(outs FPR64:$rd),
                                                (ins m.vrclass:$rs2,
                                                     ixlenimm:$sew),
                                                []>, RISCVVPseudo;
       let HasVLOp = 1, HasSEWOp = 1, BaseInstr = VFMV_S_F, WritesElement0 = 1,
           Constraints = "$rd = $rs1" in
       def PseudoVFMV_S_F # "_" # m.MX : Pseudo<(outs m.vrclass:$rd),
                                                (ins m.vrclass:$rs1, FPR32:$rs2,
                                                     GPR:$vl, ixlenimm:$sew),

 let Predicates = [HasStdExtV, HasStdExtF] in {
 foreach fvti = AllFloatVectors in {
   defvar instr = !cast<Instruction>("PseudoVFMV_F_S_" # fvti.LMul.MX);
   def : Pat<(fvti.Scalar (int_riscv_vfmv_f_s (fvti.Vector fvti.RegClass:$rs2))),
              // Floating point instructions with a scalar result will always
              // generate the result in a register of class FPR32. When dealing
              // with the f64 variant of a pattern we need to promote the FPR32
              // subregister generated by the instruction to the FPR64 base
              // register expected by the type in the pattern
              !cond(!eq(!cast<string>(fvti.ScalarRegClass),
-                       !cast<string>(FPR64)):
-                      (SUBREG_TO_REG (i32 -1),
-                                     (instr $rs2, fvti.SEW), sub_32),
+                       !cast<string>(FPR32)):
+                      (EXTRACT_SUBREG (instr $rs2, fvti.SEW), sub_32),
                    !eq(!cast<string>(fvti.ScalarRegClass),
                        !cast<string>(FPR16)):
                       (EXTRACT_SUBREG (instr $rs2, fvti.SEW), sub_16),
                    !eq(1, 1):
                       (instr $rs2, fvti.SEW))>;
 
   def : Pat<(fvti.Vector (int_riscv_vfmv_s_f (fvti.Vector fvti.RegClass:$rs1),
                          (fvti.Scalar fvti.ScalarRegClass:$rs2), GPR:$vl)),

llvm/test/CodeGen/RISCV/rvv/vfmv.f.s.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv64 -mattr=+d,+experimental-v,+experimental-zfh -target-abi lp64d -verify-machineinstrs < %s | FileCheck %s
 ; RUN: llc -mtriple=riscv32 -mattr=+d,+experimental-v,+experimental-zfh -target-abi ilp32d -verify-machineinstrs < %s | FileCheck %s
 
 declare half @llvm.riscv.vfmv.f.s.nxv1f16(<vscale x 1 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv1f16(<vscale x 1 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv1f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,mf4,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv1f16(<vscale x 1 x half> %0)
   ret half %a
 }
 
 declare half @llvm.riscv.vfmv.f.s.nxv2f16(<vscale x 2 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv2f16(<vscale x 2 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv2f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,mf2,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv2f16(<vscale x 2 x half> %0)
   ret half %a
 }
 
 declare half @llvm.riscv.vfmv.f.s.nxv4f16(<vscale x 4 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv4f16(<vscale x 4 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv4f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,m1,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv4f16(<vscale x 4 x half> %0)
   ret half %a
 }
 
 declare half @llvm.riscv.vfmv.f.s.nxv8f16(<vscale x 8 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv8f16(<vscale x 8 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv8f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,m2,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv8f16(<vscale x 8 x half> %0)
   ret half %a
 }
 
 declare half @llvm.riscv.vfmv.f.s.nxv16f16(<vscale x 16 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv16f16(<vscale x 16 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv16f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,m4,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv16f16(<vscale x 16 x half> %0)
   ret half %a
 }
 
 declare half @llvm.riscv.vfmv.f.s.nxv32f16(<vscale x 32 x half>)
 
 define half @intrinsic_vfmv.f.s_s_nxv32f16(<vscale x 32 x half> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv32f16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e16,m8,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
-; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_f
+; CHECK-NEXT:    # kill: def $f10_h killed $f10_h killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call half @llvm.riscv.vfmv.f.s.nxv32f16(<vscale x 32 x half> %0)
   ret half %a
 }
 
 declare float @llvm.riscv.vfmv.f.s.nxv1f32(<vscale x 1 x float>)
 
 define float @intrinsic_vfmv.f.s_s_nxv1f32(<vscale x 1 x float> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv1f32:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e32,mf2,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
+; CHECK-NEXT:    # kill: def $f10_f killed $f10_f killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call float @llvm.riscv.vfmv.f.s.nxv1f32(<vscale x 1 x float> %0)
   ret float %a
 }
 
 declare float @llvm.riscv.vfmv.f.s.nxv2f32(<vscale x 2 x float>)
 
 define float @intrinsic_vfmv.f.s_s_nxv2f32(<vscale x 2 x float> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv2f32:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e32,m1,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
+; CHECK-NEXT:    # kill: def $f10_f killed $f10_f killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call float @llvm.riscv.vfmv.f.s.nxv2f32(<vscale x 2 x float> %0)
   ret float %a
 }
 
 declare float @llvm.riscv.vfmv.f.s.nxv4f32(<vscale x 4 x float>)
 
 define float @intrinsic_vfmv.f.s_s_nxv4f32(<vscale x 4 x float> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv4f32:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e32,m2,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
+; CHECK-NEXT:    # kill: def $f10_f killed $f10_f killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call float @llvm.riscv.vfmv.f.s.nxv4f32(<vscale x 4 x float> %0)
   ret float %a
 }
 
 declare float @llvm.riscv.vfmv.f.s.nxv8f32(<vscale x 8 x float>)
 
 define float @intrinsic_vfmv.f.s_s_nxv8f32(<vscale x 8 x float> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv8f32:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e32,m4,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
+; CHECK-NEXT:    # kill: def $f10_f killed $f10_f killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call float @llvm.riscv.vfmv.f.s.nxv8f32(<vscale x 8 x float> %0)
   ret float %a
 }
 
 declare float @llvm.riscv.vfmv.f.s.nxv16f32(<vscale x 16 x float>)
 
 define float @intrinsic_vfmv.f.s_s_nxv16f32(<vscale x 16 x float> %0) nounwind {
 ; CHECK-LABEL: intrinsic_vfmv.f.s_s_nxv16f32:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, zero, e32,m8,ta,mu
 ; CHECK-NEXT:    vfmv.f.s fa0, v16
+; CHECK-NEXT:    # kill: def $f10_f killed $f10_f killed $f10_d
 ; CHECK-NEXT:    ret
 entry:
   %a = call float @llvm.riscv.vfmv.f.s.nxv16f32(<vscale x 16 x float> %0)
   ret float %a
 }
 
 declare double @llvm.riscv.vfmv.f.s.nxv1f64(<vscale x 1 x double>)