Skip to content

Commit 442fd1c

Browse files
committed
[SPIR-V] Fix flakiness during switch generation.
The case-list of the switches generated by this pass were not "deterministic" (based on allocation patterns). This is because the CaseList order relied on an unordered_set order. Using the sorted exit target list for those should solve the problem. Signed-off-by: Nathan Gauër <[email protected]>
1 parent 18ec885 commit 442fd1c

File tree

4 files changed

+17
-14
lines changed

4 files changed

+17
-14
lines changed

llvm/lib/Target/SPIRV/SPIRVMergeRegionExitTargets.cpp

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
#include "SPIRVSubtarget.h"
1818
#include "SPIRVTargetMachine.h"
1919
#include "SPIRVUtils.h"
20+
#include "llvm/ADT/DenseMap.h"
21+
#include "llvm/ADT/SmallPtrSet.h"
2022
#include "llvm/Analysis/LoopInfo.h"
2123
#include "llvm/CodeGen/IntrinsicLowering.h"
2224
#include "llvm/IR/CFG.h"
@@ -71,7 +73,7 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
7173
/// terminator will take.
7274
llvm::Value *createExitVariable(
7375
BasicBlock *BB,
74-
const std::unordered_map<BasicBlock *, ConstantInt *> &TargetToValue) {
76+
const DenseMap<BasicBlock *, ConstantInt *> &TargetToValue) {
7577
auto *T = BB->getTerminator();
7678
if (isa<ReturnInst>(T))
7779
return nullptr;
@@ -103,7 +105,7 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
103105

104106
/// Replaces |BB|'s branch targets present in |ToReplace| with |NewTarget|.
105107
void replaceBranchTargets(BasicBlock *BB,
106-
const std::unordered_set<BasicBlock *> ToReplace,
108+
const SmallPtrSet<BasicBlock *, 4> &ToReplace,
107109
BasicBlock *NewTarget) {
108110
auto *T = BB->getTerminator();
109111
if (isa<ReturnInst>(T))
@@ -133,7 +135,7 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
133135
bool runOnConvergenceRegionNoRecurse(LoopInfo &LI,
134136
const SPIRV::ConvergenceRegion *CR) {
135137
// Gather all the exit targets for this region.
136-
std::unordered_set<BasicBlock *> ExitTargets;
138+
SmallPtrSet<BasicBlock *, 4> ExitTargets;
137139
for (BasicBlock *Exit : CR->Exits) {
138140
for (BasicBlock *Target : gatherSuccessors(Exit)) {
139141
if (CR->Blocks.count(Target) == 0)
@@ -164,9 +166,10 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
164166

165167
// Creating one constant per distinct exit target. This will be route to the
166168
// correct target.
167-
std::unordered_map<BasicBlock *, ConstantInt *> TargetToValue;
169+
DenseMap<BasicBlock *, ConstantInt *> TargetToValue;
168170
for (BasicBlock *Target : SortedExitTargets)
169-
TargetToValue.emplace(Target, Builder.getInt32(TargetToValue.size()));
171+
TargetToValue.insert(
172+
std::make_pair(Target, Builder.getInt32(TargetToValue.size())));
170173

171174
// Creating one variable per exit node, set to the constant matching the
172175
// targeted external block.
@@ -184,12 +187,12 @@ class SPIRVMergeRegionExitTargets : public FunctionPass {
184187
}
185188

186189
// Creating the switch to jump to the correct exit target.
187-
std::vector<std::pair<BasicBlock *, ConstantInt *>> CasesList(
188-
TargetToValue.begin(), TargetToValue.end());
189-
llvm::SwitchInst *Sw =
190-
Builder.CreateSwitch(node, CasesList[0].first, CasesList.size() - 1);
191-
for (size_t i = 1; i < CasesList.size(); i++)
192-
Sw->addCase(CasesList[i].second, CasesList[i].first);
190+
llvm::SwitchInst *Sw = Builder.CreateSwitch(node, SortedExitTargets[0],
191+
SortedExitTargets.size() - 1);
192+
for (size_t i = 1; i < SortedExitTargets.size(); i++) {
193+
BasicBlock *BB = SortedExitTargets[i];
194+
Sw->addCase(TargetToValue[BB], BB);
195+
}
193196

194197
// Fix exit branches to redirect to the new exit.
195198
for (auto Exit : CR->Exits)

llvm/test/CodeGen/SPIRV/structurizer/merge-exit-break.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ while.end:
6666

6767
; CHECK: %[[#new_end]] = OpLabel
6868
; CHECK: %[[#route:]] = OpPhi %[[#int_ty]] %[[#int_1]] %[[#while_cond]] %[[#int_0]] %[[#while_body]]
69-
; CHECK: OpSwitch %[[#route]] %[[#while_end_loopexit]] 0 %[[#if_then]]
69+
; CHECK: OpSwitch %[[#route]] %[[#if_then]] 1 %[[#while_end_loopexit]]
7070
}
7171

7272
declare token @llvm.experimental.convergence.entry() #2

llvm/test/CodeGen/SPIRV/structurizer/merge-exit-convergence-in-break.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ while.end:
7575

7676
; CHECK: %[[#new_end]] = OpLabel
7777
; CHECK: %[[#route:]] = OpPhi %[[#int_ty]] %[[#int_0]] %[[#while_cond]] %[[#int_1]] %[[#tail]]
78-
; CHECK: OpSwitch %[[#route]] %[[#while_end]] 0 %[[#while_end_loopexit]]
78+
; CHECK: OpSwitch %[[#route]] %[[#while_end_loopexit]] 1 %[[#while_end]]
7979
}
8080

8181
declare token @llvm.experimental.convergence.entry() #2

llvm/test/CodeGen/SPIRV/structurizer/merge-exit-multiple-break.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ while.end:
8585

8686
; CHECK: %[[#new_end]] = OpLabel
8787
; CHECK: %[[#route:]] = OpPhi %[[#int_ty]] %[[#int_2]] %[[#while_cond]] %[[#int_0]] %[[#while_body]] %[[#int_1]] %[[#if_end]]
88-
; CHECK: OpSwitch %[[#route]] %[[#while_end_loopexit]] 1 %[[#if_then2]] 0 %[[#if_then]]
88+
; CHECK: OpSwitch %[[#route]] %[[#if_then]] 1 %[[#if_then2]] 2 %[[#while_end_loopexit]]
8989
}
9090

9191
declare token @llvm.experimental.convergence.entry() #2

0 commit comments

Comments
 (0)