[all-commits] [llvm/llvm-project] 7eb781: [RISCV] Fix a vsetvli insertion bug involving load...

Craig Topper via All-commits all-commits at lists.llvm.org
Tue Feb 1 07:37:09 PST 2022


  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 7eb781072744b31a60e82b5a5903471032d4845f
      https://github.com/llvm/llvm-project/commit/7eb781072744b31a60e82b5a5903471032d4845f
  Author: Craig Topper <craig.topper at sifive.com>
  Date:   2022-02-01 (Tue, 01 Feb 2022)

  Changed paths:
    M llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
    M llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir

  Log Message:
  -----------
  [RISCV] Fix a vsetvli insertion bug involving loads/stores.

The first phase of the analysis can avoid a vsetvli if an earlier
instruction in the block used an SEW and LMUL that when combined with
the EEW of the load/store would produce the desired EMUL. If we
avoided a vsetvli this will affect the global analysis we do in the
second phase.

The third phase where we really insert the vsetvlis needs to agree
with the first phase. If it doesn't we can insert vsetvlis that
invalidate the global analysis.

In the test case there is a VSETVLI in the preheader that sets
SEW=64 and LMUL=1. Inside the loop there is a VADD with SEW=64 and LMUL=1.
This VADD is followed by a store that wants wants SEW=32 LMUL=1/2.
Because it has EEW=32 as part of the opcode the SEW=64 LMUL=1 from the
VADD can be become EMUL=1 for the store. So the first phase determines no
vsetvli is needed.

The third phase manages CurInfo differently than BBInfo.Change from the
first phase. CurInfo is only updated when we see a vsetvli or insert
a vsetvli. This was done to allow predecessor block information from
the global analysis to be applied to multiple instructions. Since the
loop body has no vsetvli we won't update CurInfo for either the VADD
or the VSE. This prevented us from checking the store vsetvli elision
for the VSE resulting in a vsetvli SEW=32 LMUL=1/2 being emitted which
invalidated the global analysis.

To mitigate this, I've added a BBLocalInfo variable that more closely
matches the first phase propagation. This gets updated based on the
VADD and prevents emitting a vsetvli for the store like we did in the
first phase.

I wonder if we should do an earlier phase to handle the load/store case
by adding more pseudo opcodes and changing the SEW/LMUL for those
instructions before the insertion analysis. That might be more robust
than trying to guarantee two phases make the same decision.

Fixes the test from D118629.

Reviewed By: frasercrmck

Differential Revision: https://reviews.llvm.org/D118667




More information about the All-commits mailing list