Skip to content

Commit 55ac652

Browse files
authored
llvm-reduce: Do not delete convergencectrl in operand-bundles (#133858)
The IR verifier will fail if there are any convergent calls without a convergencectrl bundle, if there are any convergencectrl bundles. With the current verifier rules, we would need to drop all the instances of convergencectrl in the function as a set, and strip all the convergence token intrinsics. As such, I think it would be more appropriate to have a separate convergence reduction pass.
1 parent 222297b commit 55ac652

File tree

2 files changed

+48
-2
lines changed

2 files changed

+48
-2
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
; Check that invalid reductions aren't introduced by deleting
2+
; convergencectrl bundles in convergent functions
3+
;
4+
; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=operand-bundles --test FileCheck --test-arg --check-prefixes=CHECK,INTERESTING --test-arg %s --test-arg --input-file %s -o %t
5+
; RUN: FileCheck --check-prefixes=CHECK,RESULT %s < %t
6+
7+
; CHECK-LABEL: define float @convergentctrl_one_interesting(
8+
; INTERESTING: %interesting = call float @convergent.extern.func(
9+
; RESULT: %entry.token = call token @llvm.experimental.convergence.entry()
10+
; RESULT: %interesting = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
11+
; RESULT: %boring = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
12+
define float @convergentctrl_one_interesting(float %x, float %y) #0 {
13+
%entry.token = call token @llvm.experimental.convergence.entry()
14+
%interesting = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
15+
%boring = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
16+
%add = fadd float %interesting, %boring
17+
ret float %add
18+
}
19+
20+
; In theory we could remove the bundle here, since all convergencectrl
21+
; in the function will be removed.
22+
23+
; CHECK-LABEL: define float @convergentctrl_can_remove_all(
24+
; RESULT: %entry.token = call token @llvm.experimental.convergence.entry()
25+
; RESULT: %val = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
26+
define float @convergentctrl_can_remove_all(float %x, float %y) #0 {
27+
%entry.token = call token @llvm.experimental.convergence.entry()
28+
%val = call float @convergent.extern.func(float %x) [ "convergencectrl"(token %entry.token) ]
29+
ret float %val
30+
}
31+
32+
declare float @convergent.extern.func(float) #0
33+
declare token @llvm.experimental.convergence.entry() #1
34+
35+
attributes #0 = { convergent }
36+
attributes #1 = { convergent nocallback nofree nosync nounwind willreturn memory(none) }

llvm/tools/llvm-reduce/deltas/ReduceOperandBundles.cpp

+12-2
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,13 @@ using namespace llvm;
3131

3232
namespace {
3333

34+
/// Return true if stripping the bundle from a call will result in invalid IR.
35+
static bool shouldKeepBundleTag(uint32_t BundleTagID) {
36+
// In convergent functions using convergencectrl bundles, all convergent calls
37+
// must use the convergence bundles so don't try to remove them.
38+
return BundleTagID == LLVMContext::OB_convergencectrl;
39+
}
40+
3441
/// Given ChunksToKeep, produce a map of calls and indexes of operand bundles
3542
/// to be preserved for each call.
3643
class OperandBundleRemapper : public InstVisitor<OperandBundleRemapper> {
@@ -52,9 +59,12 @@ class OperandBundleRemapper : public InstVisitor<OperandBundleRemapper> {
5259
OperandBundlesToKeepIndexes.reserve(Call.getNumOperandBundles());
5360

5461
// Enumerate every operand bundle on this call.
55-
for (unsigned BundleIndex : seq(Call.getNumOperandBundles()))
56-
if (O.shouldKeep()) // Should we keep this one?
62+
for (unsigned BundleIndex : seq(Call.getNumOperandBundles())) {
63+
if (shouldKeepBundleTag(
64+
Call.getOperandBundleAt(BundleIndex).getTagID()) ||
65+
O.shouldKeep()) // Should we keep this one?
5766
OperandBundlesToKeepIndexes.emplace_back(BundleIndex);
67+
}
5868
}
5969
};
6070

0 commit comments

Comments
 (0)