[llvm] 1474880 - [RISCV] Use classic dataflow for VSETVLI insertion
Philip Reames via llvm-commits
llvm-commits at lists.llvm.org
Mon May 16 17:06:45 PDT 2022
Author: Philip Reames
Date: 2022-05-16T17:06:27-07:00
New Revision: 1474880353f12a5a1beb62fc873477a8a2e6afda
URL: https://github.com/llvm/llvm-project/commit/1474880353f12a5a1beb62fc873477a8a2e6afda
DIFF: https://github.com/llvm/llvm-project/commit/1474880353f12a5a1beb62fc873477a8a2e6afda.diff
LOG: [RISCV] Use classic dataflow for VSETVLI insertion
Our current implementation of the InsertVSETVLI dataflow allows phase 3 to arrive at a different block end state than the data flow in phase 1/2 computed. This arises because a block which contains instructions (e.g. load or stores) which don't consume all the incoming bits of the VL/VTYPE can be compatible with multiple incoming states. The algorithm effectively changes the SEW on such instructions, and propagates the prior state forward. As phase 3 uses the block input state for this propagation, but phase 1/2 doesn't, this can result in different block end states.
If we don't correct for it, this discrepancy can result in miscompiles. This was the source of multiple recent bugs. However, by now we have fixes for all known correctness issues.
The basic strategy we use is to insert a compensation vsetvli to bring the block state leaving the block back into consistency with the one computed. This is correct, but results in extra vsetvlis being placed at the end of blocks.
This change adjusts the phase 1/2 algorithm to propagate the incoming block state through the block, allowing the compatibility rules to modify the end state. The algorithm may need to run slightly more iterations, but the end result is consistent with what phase 3 does.
The benefit of doing this is two fold.
First, we reverse some of the code quality introductions introduced in the functional fixes.
Second, we simplify the invariants, and allow the strict assertions to be enabled. Several humans, myself included, have found it quite surprising that invariant didn't hold already, and arguably that confusion is the cause of several of our recent miscompiles in this code.
The downside to this patch is that the dataflow may require additional iterations to stabilize. In the worse case, we go from O(Edges) to O(E + UniquePaths) as the incoming state (and thus the outgoing one) can now change once for each path from the entry block.
Differential Revision: https://reviews.llvm.org/D125232
Added:
Modified:
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
Removed:
################################################################################
diff --git a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
index d7d7cc79edb49..c20fa764d510e 100644
--- a/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
@@ -373,24 +373,6 @@ class VSETVLIInfo {
return VSETVLIInfo::getUnknown();
}
- // Calculate the VSETVLIInfo visible at the end of the block assuming this
- // is the predecessor value, and Other is change for this block.
- VSETVLIInfo merge(const VSETVLIInfo &Other) const {
- assert(isValid() && "Can only merge with a valid VSETVLInfo");
-
- // Nothing changed from the predecessor, keep it.
- if (!Other.isValid())
- return *this;
-
- // If the change is compatible with the input, we won't create a VSETVLI
- // and should keep the predecessor.
- if (isCompatible(Other, /*Strict*/ true))
- return *this;
-
- // Otherwise just use whatever is in this block.
- return Other;
- }
-
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
/// Support for debugging, callable in GDB: V->dump()
LLVM_DUMP_METHOD void dump() const {
@@ -946,6 +928,7 @@ bool RISCVInsertVSETVLI::computeVLVTYPEChanges(const MachineBasicBlock &MBB) {
bool HadVectorOp = false;
BlockData &BBInfo = BlockInfo[MBB.getNumber()];
+ BBInfo.Change = BBInfo.Pred;
for (const MachineInstr &MI : MBB) {
// If this is an explicit VSETVLI or VSETIVLI, update our state.
if (isVectorConfigInstr(MI)) {
@@ -983,15 +966,11 @@ bool RISCVInsertVSETVLI::computeVLVTYPEChanges(const MachineBasicBlock &MBB) {
BBInfo.Change = VSETVLIInfo::getUnknown();
}
- // Initial exit state is whatever change we found in the block.
- BBInfo.Exit = BBInfo.Change;
- LLVM_DEBUG(dbgs() << "Initial exit state of " << printMBBReference(MBB)
- << " is " << BBInfo.Exit << "\n");
-
return HadVectorOp;
}
void RISCVInsertVSETVLI::computeIncomingVLVTYPE(const MachineBasicBlock &MBB) {
+
BlockData &BBInfo = BlockInfo[MBB.getNumber()];
BBInfo.InQueue = false;
@@ -1017,7 +996,12 @@ void RISCVInsertVSETVLI::computeIncomingVLVTYPE(const MachineBasicBlock &MBB) {
LLVM_DEBUG(dbgs() << "Entry state of " << printMBBReference(MBB)
<< " changed to " << BBInfo.Pred << "\n");
- VSETVLIInfo TmpStatus = BBInfo.Pred.merge(BBInfo.Change);
+ // Note: It's tempting to cache the state changes here, but due to the
+ // compatibility checks performed a blocks output state can change based on
+ // the input state. To cache, we'd have to add logic for finding
+ // never-compatible state changes.
+ computeVLVTYPEChanges(MBB);
+ VSETVLIInfo TmpStatus = BBInfo.Change;
// If the new exit value matches the old exit value, we don't need to revisit
// any blocks.
@@ -1306,8 +1290,15 @@ bool RISCVInsertVSETVLI::runOnMachineFunction(MachineFunction &MF) {
bool HaveVectorOp = false;
// Phase 1 - determine how VL/VTYPE are affected by the each block.
- for (const MachineBasicBlock &MBB : MF)
+ for (const MachineBasicBlock &MBB : MF) {
HaveVectorOp |= computeVLVTYPEChanges(MBB);
+ // Initial exit state is whatever change we found in the block.
+ BlockData &BBInfo = BlockInfo[MBB.getNumber()];
+ BBInfo.Exit = BBInfo.Change;
+ LLVM_DEBUG(dbgs() << "Initial exit state of " << printMBBReference(MBB)
+ << " is " << BBInfo.Exit << "\n");
+
+ }
// If we didn't find any instructions that need VSETVLI, we're done.
if (!HaveVectorOp) {
diff --git a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
index 5d8a855701741..bdaa993435589 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.ll
@@ -23,15 +23,13 @@ declare void @llvm.riscv.vse.nxv2f32(<vscale x 2 x float>, <vscale x 2 x float>*
define <vscale x 1 x double> @test1(i64 %avl, i8 zeroext %cond, <vscale x 1 x double> %a, <vscale x 1 x double> %b) nounwind {
; CHECK-LABEL: test1:
; CHECK: # %bb.0: # %entry
-; CHECK-NEXT: vsetvli a0, a0, e64, m1, ta, mu
+; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: beqz a1, .LBB0_2
; CHECK-NEXT: # %bb.1: # %if.then
; CHECK-NEXT: vfadd.vv v8, v8, v9
-; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: ret
; CHECK-NEXT: .LBB0_2: # %if.else
; CHECK-NEXT: vfsub.vv v8, v8, v9
-; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: ret
entry:
%0 = tail call i64 @llvm.riscv.vsetvli(i64 %avl, i64 3, i64 0)
@@ -56,15 +54,14 @@ if.end: ; preds = %if.else, %if.then
define <vscale x 1 x double> @test2(i64 %avl, i8 zeroext %cond, <vscale x 1 x double> %a, <vscale x 1 x double> %b) nounwind {
; CHECK-LABEL: test2:
; CHECK: # %bb.0: # %entry
-; CHECK-NEXT: vsetvli a0, a0, e64, m1, ta, mu
+; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: beqz a1, .LBB1_2
; CHECK-NEXT: # %bb.1: # %if.then
; CHECK-NEXT: vfadd.vv v9, v8, v9
-; CHECK-NEXT: j .LBB1_3
+; CHECK-NEXT: vfmul.vv v8, v9, v8
+; CHECK-NEXT: ret
; CHECK-NEXT: .LBB1_2: # %if.else
; CHECK-NEXT: vfsub.vv v9, v8, v9
-; CHECK-NEXT: .LBB1_3: # %if.end
-; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: vfmul.vv v8, v9, v8
; CHECK-NEXT: ret
entry:
@@ -183,23 +180,22 @@ define <vscale x 1 x double> @test5(i64 %avl, i8 zeroext %cond, <vscale x 1 x do
; CHECK-LABEL: test5:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: andi a2, a1, 1
-; CHECK-NEXT: vsetvli a0, a0, e64, m1, ta, mu
-; CHECK-NEXT: bnez a2, .LBB4_2
+; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
+; CHECK-NEXT: bnez a2, .LBB4_3
; CHECK-NEXT: # %bb.1: # %if.else
; CHECK-NEXT: vfsub.vv v9, v8, v9
-; CHECK-NEXT: j .LBB4_3
-; CHECK-NEXT: .LBB4_2: # %if.then
+; CHECK-NEXT: andi a0, a1, 2
+; CHECK-NEXT: beqz a0, .LBB4_4
+; CHECK-NEXT: .LBB4_2: # %if.then4
+; CHECK-NEXT: vfmul.vv v8, v9, v8
+; CHECK-NEXT: ret
+; CHECK-NEXT: .LBB4_3: # %if.then
; CHECK-NEXT: vfadd.vv v9, v8, v9
-; CHECK-NEXT: .LBB4_3: # %if.end
-; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
; CHECK-NEXT: andi a0, a1, 2
-; CHECK-NEXT: bnez a0, .LBB4_5
-; CHECK-NEXT: # %bb.4: # %if.else5
+; CHECK-NEXT: bnez a0, .LBB4_2
+; CHECK-NEXT: .LBB4_4: # %if.else5
; CHECK-NEXT: vfmul.vv v8, v8, v9
; CHECK-NEXT: ret
-; CHECK-NEXT: .LBB4_5: # %if.then4
-; CHECK-NEXT: vfmul.vv v8, v9, v8
-; CHECK-NEXT: ret
entry:
%0 = tail call i64 @llvm.riscv.vsetvli(i64 %avl, i64 3, i64 0)
%conv = zext i8 %cond to i32
@@ -242,17 +238,29 @@ define <vscale x 1 x double> @test6(i64 %avl, i8 zeroext %cond, <vscale x 1 x do
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: andi a3, a1, 1
; CHECK-NEXT: vsetvli a2, a0, e64, m1, ta, mu
-; CHECK-NEXT: bnez a3, .LBB5_2
+; CHECK-NEXT: bnez a3, .LBB5_3
; CHECK-NEXT: # %bb.1: # %if.else
; CHECK-NEXT: vfsub.vv v8, v8, v9
-; CHECK-NEXT: j .LBB5_3
-; CHECK-NEXT: .LBB5_2: # %if.then
+; CHECK-NEXT: andi a1, a1, 2
+; CHECK-NEXT: beqz a1, .LBB5_4
+; CHECK-NEXT: .LBB5_2: # %if.then4
+; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
+; CHECK-NEXT: lui a0, %hi(.LCPI5_0)
+; CHECK-NEXT: addi a0, a0, %lo(.LCPI5_0)
+; CHECK-NEXT: vlse64.v v9, (a0), zero
+; CHECK-NEXT: lui a0, %hi(.LCPI5_1)
+; CHECK-NEXT: addi a0, a0, %lo(.LCPI5_1)
+; CHECK-NEXT: vlse64.v v10, (a0), zero
+; CHECK-NEXT: vfadd.vv v9, v9, v10
+; CHECK-NEXT: lui a0, %hi(scratch)
+; CHECK-NEXT: addi a0, a0, %lo(scratch)
+; CHECK-NEXT: vse64.v v9, (a0)
+; CHECK-NEXT: j .LBB5_5
+; CHECK-NEXT: .LBB5_3: # %if.then
; CHECK-NEXT: vfadd.vv v8, v8, v9
-; CHECK-NEXT: .LBB5_3: # %if.end
-; CHECK-NEXT: vsetvli zero, a2, e64, m1, ta, mu
; CHECK-NEXT: andi a1, a1, 2
-; CHECK-NEXT: bnez a1, .LBB5_5
-; CHECK-NEXT: # %bb.4: # %if.else5
+; CHECK-NEXT: bnez a1, .LBB5_2
+; CHECK-NEXT: .LBB5_4: # %if.else5
; CHECK-NEXT: vsetvli zero, a0, e32, m1, ta, mu
; CHECK-NEXT: lui a0, %hi(.LCPI5_2)
; CHECK-NEXT: addi a0, a0, %lo(.LCPI5_2)
@@ -264,20 +272,7 @@ define <vscale x 1 x double> @test6(i64 %avl, i8 zeroext %cond, <vscale x 1 x do
; CHECK-NEXT: lui a0, %hi(scratch)
; CHECK-NEXT: addi a0, a0, %lo(scratch)
; CHECK-NEXT: vse32.v v9, (a0)
-; CHECK-NEXT: j .LBB5_6
-; CHECK-NEXT: .LBB5_5: # %if.then4
-; CHECK-NEXT: vsetvli zero, a0, e64, m1, ta, mu
-; CHECK-NEXT: lui a0, %hi(.LCPI5_0)
-; CHECK-NEXT: addi a0, a0, %lo(.LCPI5_0)
-; CHECK-NEXT: vlse64.v v9, (a0), zero
-; CHECK-NEXT: lui a0, %hi(.LCPI5_1)
-; CHECK-NEXT: addi a0, a0, %lo(.LCPI5_1)
-; CHECK-NEXT: vlse64.v v10, (a0), zero
-; CHECK-NEXT: vfadd.vv v9, v9, v10
-; CHECK-NEXT: lui a0, %hi(scratch)
-; CHECK-NEXT: addi a0, a0, %lo(scratch)
-; CHECK-NEXT: vse64.v v9, (a0)
-; CHECK-NEXT: .LBB5_6: # %if.end10
+; CHECK-NEXT: .LBB5_5: # %if.end10
; CHECK-NEXT: vsetvli zero, a2, e64, m1, ta, mu
; CHECK-NEXT: vfmul.vv v8, v8, v8
; CHECK-NEXT: ret
@@ -342,7 +337,6 @@ define <vscale x 1 x double> @test8(i64 %avl, i8 zeroext %cond, <vscale x 1 x do
; CHECK-NEXT: beqz a1, .LBB6_2
; CHECK-NEXT: # %bb.1: # %if.then
; CHECK-NEXT: vfadd.vv v8, v8, v9
-; CHECK-NEXT: vsetvli zero, s0, e64, m1, ta, mu
; CHECK-NEXT: j .LBB6_3
; CHECK-NEXT: .LBB6_2: # %if.else
; CHECK-NEXT: csrr a0, vlenb
@@ -418,7 +412,6 @@ define <vscale x 1 x double> @test9(i64 %avl, i8 zeroext %cond, <vscale x 1 x do
; CHECK-NEXT: j .LBB7_3
; CHECK-NEXT: .LBB7_2: # %if.else
; CHECK-NEXT: vfsub.vv v9, v8, v9
-; CHECK-NEXT: vsetvli zero, s0, e64, m1, ta, mu
; CHECK-NEXT: .LBB7_3: # %if.end
; CHECK-NEXT: vsetvli zero, s0, e64, m1, ta, mu
; CHECK-NEXT: vfmul.vv v8, v9, v8
diff --git a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
index 9a44fe519d30b..2d57de1e44f1c 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
+++ b/llvm/test/CodeGen/RISCV/rvv/vsetvli-insert-crossbb.mir
@@ -377,7 +377,6 @@ body: |
; CHECK-NEXT: [[PHI:%[0-9]+]]:vr = PHI [[PseudoVADD_VV_M1_]], %bb.1, [[PseudoVSUB_VV_M1_]], %bb.2
; CHECK-NEXT: [[PseudoVMV_X_S_M1_:%[0-9]+]]:gpr = PseudoVMV_X_S_M1 [[PHI]], 6 /* e64 */, implicit $vtype
; CHECK-NEXT: $x10 = COPY [[PseudoVMV_X_S_M1_]]
- ; CHECK-NEXT: dead $x0 = PseudoVSETVLIX0 killed $x0, 88 /* e64, m1, ta, mu */, implicit-def $vl, implicit-def $vtype, implicit $vl
; CHECK-NEXT: PseudoRET implicit $x10
bb.0.entry:
successors: %bb.2(0x30000000), %bb.1(0x50000000)
@@ -437,7 +436,7 @@ body: |
; CHECK-NEXT: [[COPY1:%[0-9]+]]:vr = COPY $v9
; CHECK-NEXT: [[COPY2:%[0-9]+]]:vr = COPY $v8
; CHECK-NEXT: [[COPY3:%[0-9]+]]:gpr = COPY $x10
- ; CHECK-NEXT: [[PseudoVSETVLI:%[0-9]+]]:gprnox0 = PseudoVSETVLI [[COPY]], 88 /* e64, m1, ta, mu */, implicit-def $vl, implicit-def $vtype
+ ; CHECK-NEXT: $x0 = PseudoVSETVLI [[COPY]], 88 /* e64, m1, ta, mu */, implicit-def $vl, implicit-def $vtype
; CHECK-NEXT: [[COPY4:%[0-9]+]]:gpr = COPY $x0
; CHECK-NEXT: BEQ [[COPY3]], [[COPY4]], %bb.2
; CHECK-NEXT: PseudoBR %bb.1
@@ -446,14 +445,12 @@ body: |
; CHECK-NEXT: successors: %bb.3(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[PseudoVADD_VV_M1_:%[0-9]+]]:vr = PseudoVADD_VV_M1 [[COPY2]], [[COPY1]], $noreg, 6 /* e64 */, implicit $vl, implicit $vtype
- ; CHECK-NEXT: dead $x0 = PseudoVSETVLI [[PseudoVSETVLI]], 88 /* e64, m1, ta, mu */, implicit-def $vl, implicit-def $vtype
; CHECK-NEXT: PseudoBR %bb.3
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2.if.else:
; CHECK-NEXT: successors: %bb.3(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[PseudoVSUB_VV_M1_:%[0-9]+]]:vr = PseudoVSUB_VV_M1 [[COPY2]], [[COPY1]], $noreg, 6 /* e64 */, implicit $vl, implicit $vtype
- ; CHECK-NEXT: dead $x0 = PseudoVSETVLI [[PseudoVSETVLI]], 88 /* e64, m1, ta, mu */, implicit-def $vl, implicit-def $vtype
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.3.if.end:
; CHECK-NEXT: [[PHI:%[0-9]+]]:vr = PHI [[PseudoVADD_VV_M1_]], %bb.1, [[PseudoVSUB_VV_M1_]], %bb.2
@@ -849,7 +846,6 @@ body: |
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: %mask:vr = PseudoVMANDN_MM_MF8 %t6, %t3, -1, 0 /* e8 */, implicit $vl, implicit $vtype
; CHECK-NEXT: %t2:gpr = COPY $x0
- ; CHECK-NEXT: dead $x0 = PseudoVSETVLIX0 killed $x0, 69 /* e8, mf8, ta, mu */, implicit-def $vl, implicit-def $vtype, implicit $vl
; CHECK-NEXT: BEQ %a, %t2, %bb.3
; CHECK-NEXT: PseudoBR %bb.2
; CHECK-NEXT: {{ $}}
@@ -857,6 +853,7 @@ body: |
; CHECK-NEXT: successors: %bb.3(0x80000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: $v0 = COPY %mask
+ ; CHECK-NEXT: dead $x0 = PseudoVSETVLIX0 killed $x0, 69 /* e8, mf8, ta, mu */, implicit-def $vl, implicit-def $vtype, implicit $vl
; CHECK-NEXT: early-clobber %t0:vrnov0 = PseudoVLUXEI64_V_M1_MF8_MASK %t5, killed %inaddr, %idxs, $v0, -1, 3 /* e8 */, 1, implicit $vl, implicit $vtype
; CHECK-NEXT: %ldval:vr = COPY %t0
; CHECK-NEXT: PseudoBR %bb.3
@@ -864,6 +861,7 @@ body: |
; CHECK-NEXT: bb.3:
; CHECK-NEXT: %stval:vr = PHI %t4, %bb.1, %ldval, %bb.2
; CHECK-NEXT: $v0 = COPY %mask
+ ; CHECK-NEXT: dead $x0 = PseudoVSETVLIX0 killed $x0, 69 /* e8, mf8, ta, mu */, implicit-def $vl, implicit-def $vtype, implicit $vl
; CHECK-NEXT: PseudoVSOXEI64_V_M1_MF8_MASK killed %stval, killed %b, %idxs, $v0, -1, 3 /* e8 */, implicit $vl, implicit $vtype
; CHECK-NEXT: PseudoRET
bb.0:
More information about the llvm-commits
mailing list