Skip to content

Commit fb368a1

Browse files
authored
[SYCL] Fix propagation of aspects from global variables (#11173)
Before this PR, the `pf-wg-atomic64.cpp` (from this PR) fails because the `atomic64` aspect was not propagated to the calling kernel. The reason is: - With opaque pointers disabled, since the `this` parameter of the constructor of `atomic_ref` would have a type of `atomic_ref*`, the call to the constructor of `atomic_ref` would be enough to propagate the `atomic64` aspect to the kernel. With opaque pointers enabled, this would not be the case: the `this` parameter would just be a `ptr`. - Additionally, for some reason I am unsure of, the code generated for `parallel_for_work_group` (and not, say, for `parallel_for`) has the pointer for `feature` generated as a global variable instead of generated from an `alloca` instruction. If there were an `alloca` of an `atomic_ref`, there would be no problem. Now, when examining the operands of instructions in `SYCLPropagateAspectsPass`, we now consider if the operand is a global variable, and if so, propagate the type is points to instead of propagating the pointer type.
1 parent 491fb26 commit fb368a1

File tree

3 files changed

+65
-8
lines changed

3 files changed

+65
-8
lines changed

llvm/lib/SYCLLowerIR/SYCLPropagateAspectsUsage.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -239,21 +239,24 @@ AspectsSetTy getAspectsUsedByInstruction(const Instruction &I,
239239
ReturnType = AI->getAllocatedType();
240240
}
241241
AspectsSetTy Result = getAspectsFromType(ReturnType, Types);
242-
for (const auto &OperandIt : I.operands()) {
243-
const AspectsSetTy &Aspects =
244-
getAspectsFromType(OperandIt->getType(), Types);
242+
auto AddAspectsFromType = [&](Type *Ty) {
243+
const AspectsSetTy &Aspects = getAspectsFromType(Ty, Types);
245244
Result.insert(Aspects.begin(), Aspects.end());
245+
};
246+
for (const auto &OperandIt : I.operands()) {
247+
if (const auto *GV =
248+
dyn_cast<const GlobalValue>(OperandIt->stripPointerCasts()))
249+
AddAspectsFromType(GV->getValueType());
250+
else
251+
AddAspectsFromType(OperandIt->getType());
246252
}
247253

248254
// Opaque pointer arguments may hide types of pointer arguments until elements
249255
// inside the types are accessed through a GEP instruction. However, this will
250256
// not be caught by the operands check above, so we must extract the
251257
// information directly from the GEP.
252-
if (auto *GEPI = dyn_cast<const GetElementPtrInst>(&I)) {
253-
const AspectsSetTy &Aspects =
254-
getAspectsFromType(GEPI->getSourceElementType(), Types);
255-
Result.insert(Aspects.begin(), Aspects.end());
256-
}
258+
if (auto *GEPI = dyn_cast<const GetElementPtrInst>(&I))
259+
AddAspectsFromType(GEPI->getSourceElementType());
257260

258261
if (const MDNode *InstApsects = I.getMetadata("sycl_used_aspects")) {
259262
for (const MDOperand &MDOp : InstApsects->operands()) {
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
; This tests ensures that the aspects of a global variable are propagated.
2+
; RUN: opt -passes=sycl-propagate-aspects-usage < %s -S | FileCheck %s
3+
4+
%struct.StructWithAspect = type { i32 }
5+
@global = internal addrspace(3) global %struct.StructWithAspect undef, align 8
6+
7+
declare void @external(ptr addrspace(4))
8+
9+
; CHECK: spir_kernel void @foo() !sycl_used_aspects ![[MDID:[0-9]+]]
10+
define spir_kernel void @foo() {
11+
%res = load ptr addrspace(3), ptr addrspace(3) @global
12+
ret void
13+
}
14+
15+
; CHECK: spir_kernel void @bar() !sycl_used_aspects ![[MDID]]
16+
define spir_kernel void @bar() {
17+
call void @external(ptr addrspace(4) addrspacecast(ptr addrspace(3) @global to ptr addrspace(4)))
18+
ret void
19+
}
20+
21+
!sycl_types_that_use_aspects = !{!1}
22+
!sycl_aspects = !{!2}
23+
24+
!1 = !{!"struct.StructWithAspect", i32 6}
25+
!2 = !{!"fp64", i32 6}
26+
27+
; CHECK: ![[MDID]] = !{i32 6}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// DISABLED: aspect-atomic64
2+
// RUN: %{build} -o %t.out
3+
// RUN: %{run} %t.out
4+
5+
#include <sycl/sycl.hpp>
6+
using namespace sycl;
7+
8+
using AtomicRefT =
9+
atomic_ref<unsigned long long, memory_order::relaxed, memory_scope::device>;
10+
11+
int main() {
12+
queue q;
13+
auto *p = malloc_shared<unsigned long long>(1, q);
14+
try {
15+
q.submit([&](sycl::handler &cgh) {
16+
cgh.parallel_for_work_group(range{1}, range{1}, [=](group<1>) {
17+
AtomicRefT feature(*p);
18+
feature += 42;
19+
});
20+
}).wait();
21+
} catch (sycl::exception &e) {
22+
if (e.code() != sycl::errc::kernel_not_supported)
23+
throw;
24+
std::cout << "Caught right exception: " << e.what() << "\n";
25+
return 0;
26+
}
27+
}

0 commit comments

Comments
 (0)