[RISCV] Optimize scalable frame offset calculation when VLEN is precisely known

[reames]

Summary

When we have a precisely known VLEN, we can replace runtime usage of VLENB with compile time constants. This converts offsets involving both fixed and scalable components into fixed offsets. The result is that we avoid the csr read of vlenb, and can often fold the multiply as well.

[kito-cheng]

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

7

Should be SPILL-O2-VLEN256?

90

I thought there should not appear any read vlenb here? since according the description I expect we could just use 256 here?

[kito-cheng]

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

6

And this also an old problem here, should we emit a warring or error when zvl*b info is mismatch with -riscv-v-vector-bits-min=? min vlen is 128 according the zvl128b (which implied by v), but we force that become 256 by -riscv-v-vector-bits-min=, that became an issue when we use Tag_RISCV_arch* to check the expected minimal vlen requirement for the object?

• There is still controversial about how to interpret Tag_RISCV_arch, but eventually will resolved that by some way.

[kito-cheng]

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

90

my bad, it's separated patch, I saw https://reviews.llvm.org/D137593 now

[reames]

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

6

There's no conflict here. Zvl128b is a minimum, not a maximum. As such, specifying a larger VLEN via -riscv-v-vector-bits-min does not conflict.

There's a separate question of whether the -riscv-v-vector-bits-min should effect the attributes, but that's well out of scope for this patch.

7

This is a typo, and I'll fix.

[reames]

Fix test line per reviewer comment

[asb]

This looks good to me.

[reames]

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

98

The offset here is wrong. I missed a factor of 8 divide in my code which exists in the original getVLENFactoredAmount path. It's not really clear to me where that factor comes from, but what's here is definitely wrong in this test. Updated version forthcoming.

[reames]

Fix bug around missing divide by 8.

I have to admit I don't know why that divide is needed. It parallels the code in getVLENFactoredAmount, but it's not clear to me where the factor of 8 comes from.

[craig.topper]

In D137591#3919658, @reames wrote:

Fix bug around missing divide by 8.

I have to admit I don't know why that divide is needed. It parallels the code in getVLENFactoredAmount, but it's not clear to me where the factor of 8 comes from.

I think all scalable vector stack objects are rounded up to be a multiple of 8 by this code in RISCVFrameLowering::assignRVVStackObjectOffsets

// If the data type is the fractional vector type, reserve one vector        
// register for it.                                                          
if (ObjectSize < 8)                                                          
  ObjectSize = 8;

I think this is done because there is no fractional whole register load/store. The divide by 8 is trying to figure out how many whole registers there are.

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

185

I'm sure you copy and pasted this from somewhere else, but this message isn't great. It should be something like "Scalable offset is not a multiple of a single vector size."

[reames]

Address reviewer comment on assert message

[reames]

In D137591#3919786, @craig.topper wrote:

In D137591#3919658, @reames wrote:

Fix bug around missing divide by 8.

I have to admit I don't know why that divide is needed. It parallels the code in getVLENFactoredAmount, but it's not clear to me where the factor of 8 comes from.

I think all scalable vector stack objects are rounded up to be a multiple of 8 by this code in RISCVFrameLowering::assignRVVStackObjectOffsets

// If the data type is the fractional vector type, reserve one vector        
// register for it.                                                          
if (ObjectSize < 8)                                                          
  ObjectSize = 8;

I think this is done because there is no fractional whole register load/store. The divide by 8 is trying to figure out how many whole registers there are.

The bit I don't understand is the interpretation of object size for a scalable value. Why is "8" the right number for a single VLEN sized register?

[reames]

ping

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp

 bool RISCVRegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
                                             int SPAdj, unsigned FIOperandNum,
                                             RegScavenger *RS) const {
   assert(SPAdj == 0 && "Unexpected non-zero SPAdj value");
 
   MachineInstr &MI = *II;
   MachineFunction &MF = *MI.getParent()->getParent();
   MachineRegisterInfo &MRI = MF.getRegInfo();
-  const RISCVInstrInfo *TII = MF.getSubtarget<RISCVSubtarget>().getInstrInfo();
+  const RISCVSubtarget &ST = MF.getSubtarget<RISCVSubtarget>();
+  const RISCVInstrInfo *TII = ST.getInstrInfo();
   DebugLoc DL = MI.getDebugLoc();
 
   int FrameIndex = MI.getOperand(FIOperandNum).getIndex();
   Register FrameReg;
   StackOffset Offset =
       getFrameLowering(MF)->getFrameIndexReference(MF, FrameIndex, FrameReg);
   bool IsRVVSpill = RISCV::isRVVSpill(MI);
   if (!IsRVVSpill)
     Offset += StackOffset::getFixed(MI.getOperand(FIOperandNum + 1).getImm());
 
+  if (Offset.getScalable() &&
+      ST.getRealMinVLen() == ST.getRealMaxVLen()) {
+    // For an exact VLEN value, scalable offsets become constant and thus
+    // can be converted entirely into fixed offsets.
+    int64_t FixedValue = Offset.getFixed();
+    int64_t ScalableValue = Offset.getScalable();
+    assert(ScalableValue % 8 == 0 &&
+           "Scalable offset is not a multiple of a single vector size.");
+    int64_t NumOfVReg = ScalableValue / 8;
+    int64_t VLENB = ST.getRealMinVLen() / 8;
+    Offset = StackOffset::getFixed(FixedValue + NumOfVReg * VLENB);
+  }
+
   if (!isInt<32>(Offset.getFixed())) {
     report_fatal_error(
         "Frame offsets outside of the signed 32-bit range not supported");
   }
 
   MachineBasicBlock &MBB = *MI.getParent();
   bool FrameRegIsKill = false;
 

llvm/test/CodeGen/RISCV/rvv/rv64-spill-vector-csr.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv64 -mattr=+v,+d -mattr=+d -O0 < %s \
 ; RUN:    | FileCheck --check-prefix=SPILL-O0 %s
 ; RUN: llc -mtriple=riscv64 -mattr=+v,+d -mattr=+d -O2 < %s \
 ; RUN:    | FileCheck --check-prefix=SPILL-O2 %s
+; RUN: llc -mtriple=riscv64 -mattr=+v,+d -mattr=+d -riscv-v-vector-bits-max=128 -O2 < %s \
+; RUN:    | FileCheck --check-prefix=SPILL-O2-VLEN128 %s
+
 
 @.str = private unnamed_addr constant [6 x i8] c"hello\00", align 1
 
 define <vscale x 1 x double> @foo(<vscale x 1 x double> %a, <vscale x 1 x double> %b, <vscale x 1 x double> %c, i64 %gvl) nounwind
 ; SPILL-O0-LABEL: foo:
 ; SPILL-O0:       # %bb.0:
 ; SPILL-O0-NEXT:    addi sp, sp, -48
 ; SPILL-O0-NEXT:    sd ra, 40(sp) # 8-byte Folded Spill

 ; SPILL-O2-NEXT:    vfadd.vv v8, v9, v8
 ; SPILL-O2-NEXT:    csrr a0, vlenb
 ; SPILL-O2-NEXT:    slli a0, a0, 1
 ; SPILL-O2-NEXT:    add sp, sp, a0
 ; SPILL-O2-NEXT:    ld ra, 24(sp) # 8-byte Folded Reload
 ; SPILL-O2-NEXT:    ld s0, 16(sp) # 8-byte Folded Reload
 ; SPILL-O2-NEXT:    addi sp, sp, 32
 ; SPILL-O2-NEXT:    ret
+;
+; SPILL-O2-VLEN128-LABEL: foo:
+; SPILL-O2-VLEN128:       # %bb.0:
+; SPILL-O2-VLEN128-NEXT:    addi sp, sp, -32
+; SPILL-O2-VLEN128-NEXT:    sd ra, 24(sp) # 8-byte Folded Spill
+; SPILL-O2-VLEN128-NEXT:    sd s0, 16(sp) # 8-byte Folded Spill
+; SPILL-O2-VLEN128-NEXT:    csrr a1, vlenb
+; SPILL-O2-VLEN128-NEXT:    slli a1, a1, 1
+; SPILL-O2-VLEN128-NEXT:    sub sp, sp, a1
+; SPILL-O2-VLEN128-NEXT:    mv s0, a0
+; SPILL-O2-VLEN128-NEXT:    addi a1, sp, 16
+; SPILL-O2-VLEN128-NEXT:    vs1r.v v8, (a1) # Unknown-size Folded Spill
+; SPILL-O2-VLEN128-NEXT:    vsetvli zero, a0, e64, m1, ta, ma
+; SPILL-O2-VLEN128-NEXT:    vfadd.vv v9, v8, v9
+; SPILL-O2-VLEN128-NEXT:    addi a0, sp, 32
+; SPILL-O2-VLEN128-NEXT:    vs1r.v v9, (a0) # Unknown-size Folded Spill
+; SPILL-O2-VLEN128-NEXT:    lui a0, %hi(.L.str)
+; SPILL-O2-VLEN128-NEXT:    addi a0, a0, %lo(.L.str)
+; SPILL-O2-VLEN128-NEXT:    call puts@plt
+; SPILL-O2-VLEN128-NEXT:    vsetvli zero, s0, e64, m1, ta, ma
+; SPILL-O2-VLEN128-NEXT:    addi a0, sp, 32
+; SPILL-O2-VLEN128-NEXT:    vl1r.v v8, (a0) # Unknown-size Folded Reload
+; SPILL-O2-VLEN128-NEXT:    addi a0, sp, 16
+; SPILL-O2-VLEN128-NEXT:    vl1r.v v9, (a0) # Unknown-size Folded Reload
+; SPILL-O2-VLEN128-NEXT:    vfadd.vv v8, v9, v8
+; SPILL-O2-VLEN128-NEXT:    csrr a0, vlenb
+; SPILL-O2-VLEN128-NEXT:    slli a0, a0, 1
+; SPILL-O2-VLEN128-NEXT:    add sp, sp, a0
+; SPILL-O2-VLEN128-NEXT:    ld ra, 24(sp) # 8-byte Folded Reload
+; SPILL-O2-VLEN128-NEXT:    ld s0, 16(sp) # 8-byte Folded Reload
+; SPILL-O2-VLEN128-NEXT:    addi sp, sp, 32
+; SPILL-O2-VLEN128-NEXT:    ret
 {
    %x = call <vscale x 1 x double> @llvm.riscv.vfadd.nxv1f64.nxv1f64(<vscale x 1 x double> undef, <vscale x 1 x double> %a, <vscale x 1 x double> %b, i64 %gvl)
    %call = call signext i32 @puts(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.str, i64 0, i64 0))
    %z = call <vscale x 1 x double> @llvm.riscv.vfadd.nxv1f64.nxv1f64(<vscale x 1 x double> undef, <vscale x 1 x double> %a, <vscale x 1 x double> %x, i64 %gvl)
    ret <vscale x 1 x double> %z
 }
 
 declare <vscale x 1 x double> @llvm.riscv.vfadd.nxv1f64.nxv1f64(<vscale x 1 x double> %passthru, <vscale x 1 x double> %a, <vscale x 1 x double> %b, i64 %gvl)
 declare i32 @puts(i8*);