Skip to content

Commit 1977533

Browse files
authored
[SYCLLowerIR] Fix a hierarchical parallelism bug (#20550)
There is a bug in my earlier PR #20484. When encountering an indirect call to a `parallel_for_work_item` function, the range of instructions that will be subjected to the lowering pass must be closed, whereas the current code simply skips over this instruction as its iterating the instructions of the basic block which effectively keeps extending the range. There is an E2E test that reflects this bug where it used to hang before this fix but so far I'm not aware of an LLVM-IR snippet of code that triggers it so for the moment, I'm just adding a SYCL-level test.
1 parent c8e3310 commit 1977533

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

llvm/lib/SYCLLowerIR/LowerWGScope.cpp

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -851,14 +851,22 @@ PreservedAnalyses SYCLLowerWGScopePass::run(Function &F,
851851
}
852852
continue;
853853
}
854-
// In addition to an instruction not having side effects, we end the range
855-
// if the instruction is a call that contains, possibly several layers
856-
// down the stack, a call to a parallel_for_work_item. Such calls should
857-
// not be subject to lowering since they must be executed by every work
858-
// item.
854+
// We also split the range if the instruction is a call that contains,
855+
// possibly several layers down the stack, a call to a
856+
// parallel_for_work_item. Such calls should not be subject to lowering
857+
// since they must be executed by every work item.
859858
const CallInst *Call = dyn_cast<CallInst>(I);
860-
if (!mayHaveSideEffects(I) ||
861-
(Call && hasCallToAFuncWithPFWIMetadata(*Call->getCalledFunction())))
859+
if (Call && hasCallToAFuncWithPFWIMetadata(*Call->getCalledFunction())) {
860+
if (First) {
861+
assert(Last && "range must have been closed 1");
862+
Ranges.push_back(InstrRange{First, Last});
863+
First = nullptr;
864+
Last = nullptr;
865+
}
866+
continue;
867+
}
868+
869+
if (!mayHaveSideEffects(I))
862870
continue;
863871
LLVM_DEBUG(llvm::dbgs() << "+++ Side effects: " << *I << "\n");
864872
if (!First)

sycl/test-e2e/HierPar/hier_par_indirect.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ int main(int argc, char **argv) {
4848
wGroup.parallel_for_work_item([&](sycl::h_item<1> index) {});
4949
}));
5050
}).wait();
51+
// Also try an example of a work-group scope variable being used in work item
52+
// scope
53+
q.submit([&](sycl::handler &cgh) {
54+
cgh.parallel_for_work_group(
55+
sycl::range<1>{1}, sycl::range<1>{128}, ([=](sycl::group<1> wGroup) {
56+
int data;
57+
foo(wGroup);
58+
wGroup.parallel_for_work_item(
59+
[&](sycl::h_item<1> index) { data = 0; });
60+
}));
61+
}).wait();
5162

5263
std::cout << "test passed" << std::endl;
5364
return 0;

0 commit comments

Comments
 (0)