[RISCV] Improve the condition of hasRVVFrameObject.

[jacquesguan]

Summary

Now the function hasRVVFrameObject is using hasVInstructions() as the return value. This patch changed it to two parts judgement. Firstly, check if there is any frame object that is scalable vector. If there is no scalable vector frame object, then check if there is any V instruction in the function.

[craig.topper]

Is it possible write a test for this?

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

951–952

Does this comment need to be updated?

[jacquesguan]

Add test case and update the comment.

[jacquesguan]

In D118026#3266530, @craig.topper wrote:

Is it possible write a test for this?

I add a case to show the difference when the function will not use any rvv.

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

951–952

Done.

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

Is it possible to write a function that contains nothing but whole register loads/stores/moves such that no instruction access VL but needs to spill a register?

[jacquesguan]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

I am not sure about this, I think maybe it's not possible to generate independent whole register instructions. Do you have any good opinion about the condition?

[kito-cheng]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

Maybe check any vector register are appear in the function either use or def?

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

This generates a single whole register load

#include <riscv_vector.h>

vfloat32m4_t foo(vfloat32m4_t *a) {
  return *a;
}

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

That was only meant as an example of how register instrutions can be generated. Not that it creates scalable frame object.

[jacquesguan]

Address comment.

[jacquesguan]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

976

Done, I changed the condition to check if there ia any VR or VTYPE.

[jacquesguan]

ping.

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

965

This sentence doesn't make sense. Isn't this checking for the existence of an RVV frame object directly?

978

Does RISCV::VRRegClass.contains work for virtual registers?

[jacquesguan]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

978

NO, what do you think is a good way to check if there is any VR?

[craig.topper]

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

978

I think using MachineRegisterInfo::getRegClass

llvm/lib/Target/RISCV/RISCVFrameLowering.cpp

 //===-- RISCVFrameLowering.cpp - RISCV Frame Information ------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 static bool hasRVVFrameObject(const MachineFunction &MF) {
   // Originally, the function will scan all the stack objects to check whether
   // if there is any scalable vector object on the stack or not. However, it
   // causes errors in the register allocator. In issue 53016, it returns false
   // before RA because there is no RVV stack objects. After RA, it returns true
   // because there are spilling slots for RVV values during RA. It will not
   // reserve BP during register allocation and generate BP access in the PEI
   // pass due to the inconsistent behavior of the function.
   //
-  // The function is changed to use hasVInstructions() as the return value. It
-  // is not precise, but it can make the register allocation correct.
+  // The function is changed to check if there is any rvv frame object or any
+  // rvv instruction. It is not precise, but it can make the register allocation
+  // correct.
   //
   // FIXME: Find a better way to make the decision or revisit the solution in
   // D103622.
   //
   // Refer to https://github.com/llvm/llvm-project/issues/53016.
-  return MF.getSubtarget<RISCVSubtarget>().hasVInstructions();
+
+  // Won't have RVV frame object without V/Zve* extensions.
+  if (!MF.getSubtarget<RISCVSubtarget>().hasVInstructions())
+    return false;
+
+  // If there is scalable vector, there must be RVV frame object.
+  const MachineFrameInfo &MFI = MF.getFrameInfo();
+  for (int I = 0, E = MFI.getObjectIndexEnd(); I != E; ++I)
+    if (MFI.getStackID(I) == TargetStackID::ScalableVector)
+      return true;
+
+  // If there is any RVV instruction in the function, there might be RVV frame
+  // object.
+  for (const auto &MBB : MF)
+    for (const auto &MI : MBB)
+      for (const auto &MO : MI.operands())
+        if (MO.isReg()) {
+          auto Reg = MO.getReg();
+          if (Reg == RISCV::VTYPE || RISCV::VRRegClass.contains(Reg) ||
+              RISCV::VRM2RegClass.contains(Reg) ||
+              RISCV::VRM4RegClass.contains(Reg) ||
+              RISCV::VRM8RegClass.contains(Reg))
+            return true;
+        }
+
+  // Otherwise, there should not be any RVV frame object.
+  return false;
 }
 
 // Not preserve stack space within prologue for outgoing variables when the
 // function contains variable size objects or there are vector objects accessed
 // by the frame pointer.
 // Let eliminateCallFramePseudoInstr preserve stack space for it.
 bool RISCVFrameLowering::hasReservedCallFrame(const MachineFunction &MF) const {
   return !MF.getFrameInfo().hasVarSizedObjects() &&

llvm/test/CodeGen/RISCV/rvv/reg-alloc-reserve-bp.ll

entry:
   store <8 x float> %add, <8 x float>* %1, align 32
   call void @llvm.lifetime.end.p0i8(i64 40, i8* nonnull %0)
   ret void
 }
 
 define void @foo1(i32* nocapture noundef %p1) {
 ; CHECK-LABEL: foo1:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    addi sp, sp, -192
-; CHECK-NEXT:    .cfi_def_cfa_offset 192
-; CHECK-NEXT:    sd ra, 184(sp) # 8-byte Folded Spill
-; CHECK-NEXT:    sd s0, 176(sp) # 8-byte Folded Spill
-; CHECK-NEXT:    sd s1, 168(sp) # 8-byte Folded Spill
-; CHECK-NEXT:    sd s2, 160(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    addi sp, sp, -128
+; CHECK-NEXT:    .cfi_def_cfa_offset 128
+; CHECK-NEXT:    sd ra, 120(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    sd s0, 112(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    sd s1, 104(sp) # 8-byte Folded Spill
 ; CHECK-NEXT:    .cfi_offset ra, -8
 ; CHECK-NEXT:    .cfi_offset s0, -16
 ; CHECK-NEXT:    .cfi_offset s1, -24
-; CHECK-NEXT:    .cfi_offset s2, -32
-; CHECK-NEXT:    addi s0, sp, 192
+; CHECK-NEXT:    addi s0, sp, 128
 ; CHECK-NEXT:    .cfi_def_cfa s0, 0
 ; CHECK-NEXT:    andi sp, sp, -64
-; CHECK-NEXT:    mv s1, sp
-; CHECK-NEXT:    mv s2, a0
+; CHECK-NEXT:    mv s1, a0
 ; CHECK-NEXT:    fld ft0, 0(a0)
-; CHECK-NEXT:    fsd ft0, 56(s1) # 8-byte Folded Spill
-; CHECK-NEXT:    addi sp, sp, -16
-; CHECK-NEXT:    addi t0, s1, 64
+; CHECK-NEXT:    fsd ft0, 56(sp) # 8-byte Folded Spill
+; CHECK-NEXT:    addi t0, sp, 64
 ; CHECK-NEXT:    li a0, 1
 ; CHECK-NEXT:    li a1, 2
 ; CHECK-NEXT:    li a2, 3
 ; CHECK-NEXT:    li a3, 4
 ; CHECK-NEXT:    li a4, 5
 ; CHECK-NEXT:    li a5, 6
 ; CHECK-NEXT:    li a6, 7
 ; CHECK-NEXT:    li a7, 8
 ; CHECK-NEXT:    sd t0, 0(sp)
 ; CHECK-NEXT:    call bar@plt
-; CHECK-NEXT:    addi sp, sp, 16
-; CHECK-NEXT:    fld ft0, 0(s2)
-; CHECK-NEXT:    fld ft1, 56(s1) # 8-byte Folded Reload
+; CHECK-NEXT:    fld ft0, 0(s1)
+; CHECK-NEXT:    fld ft1, 56(sp) # 8-byte Folded Reload
 ; CHECK-NEXT:    fadd.d ft0, ft1, ft0
-; CHECK-NEXT:    fsd ft0, 0(s2)
-; CHECK-NEXT:    addi sp, s0, -192
-; CHECK-NEXT:    ld ra, 184(sp) # 8-byte Folded Reload
-; CHECK-NEXT:    ld s0, 176(sp) # 8-byte Folded Reload
-; CHECK-NEXT:    ld s1, 168(sp) # 8-byte Folded Reload
-; CHECK-NEXT:    ld s2, 160(sp) # 8-byte Folded Reload
-; CHECK-NEXT:    addi sp, sp, 192
+; CHECK-NEXT:    fsd ft0, 0(s1)
+; CHECK-NEXT:    addi sp, s0, -128
+; CHECK-NEXT:    ld ra, 120(sp) # 8-byte Folded Reload
+; CHECK-NEXT:    ld s0, 112(sp) # 8-byte Folded Reload
+; CHECK-NEXT:    ld s1, 104(sp) # 8-byte Folded Reload
+; CHECK-NEXT:    addi sp, sp, 128
 ; CHECK-NEXT:    ret
 entry:
   %vla = alloca [10 x i32], align 64
   %0 = bitcast [10 x i32]* %vla to i8*
   call void @llvm.lifetime.start.p0i8(i64 40, i8* nonnull %0)
   %1 = bitcast i32* %p1 to double*
   %2 = load double, double* %1, align 32
   %arraydecay = getelementptr inbounds [10 x i32], [10 x i32]* %vla, i64 0, i64 0