[llvm] [AMDGPU] Defaults for missing dimensions in SYCL required wg size (PR #68872)

Jakub Chlanda via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 26 03:25:58 PDT 2023


================
@@ -317,10 +317,38 @@ static bool processUse(CallInst *CI, bool IsV5OrAbove) {
   return MadeChange;
 }
 
+// SYCL allows required work-group size attribute to be partially specified
+// (not all three dimensions), provide a default value (1) for the missing
+// dimensions.
+static void updateSYCLreqdWorkGroupMD(Function &F) {
+  auto *Node = F.getMetadata("reqd_work_group_size");
+  if (!Node || Node->getNumOperands() == 3)
+    return;
+
+  auto &Context = F.getContext();
+  SmallVector<uint64_t, 3> RWGS;
+  for (auto &Op : Node->operands())
+    RWGS.push_back(mdconst::extract<ConstantInt>(Op)->getZExtValue());
+  while (RWGS.size() != 3)
+    RWGS.push_back(1);
+
+  llvm::Metadata *RWGSArgs[] = {
+      llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+          llvm::IntegerType::get(Context, 32), llvm::APInt(32, RWGS[0]))),
+      llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+          llvm::IntegerType::get(Context, 32), llvm::APInt(32, RWGS[1]))),
+      llvm::ConstantAsMetadata::get(llvm::ConstantInt::get(
+          llvm::IntegerType::get(Context, 32), llvm::APInt(32, RWGS[2])))};
+  F.setMetadata("reqd_work_group_size", llvm::MDNode::get(Context, RWGSArgs));
+}
 
 // TODO: Move makeLIDRangeMetadata usage into here. Seem to not get
 // TargetPassConfig for subtarget.
 bool AMDGPULowerKernelAttributes::runOnModule(Module &M) {
+  for (auto &F : M)
+    if (F.hasFnAttribute("sycl-module-id"))
----------------
jchlanda wrote:

Sorry, I should have been more clear.

> What's an error condition? That verifier is just checking the final output parses?

No, OpenCL mandates all 3 dimensions are set, the verifier, rightly so, expects 3 elements and errors out on sub 3 elements. It's just that the same metadata now can be created through either SYCL, or OpenCL, and SYCL happens to relax the restriction.

> But you fixed that by adjusting the metadata to match?

Yes, with this patch the condition is also true for SYCL's required work group size (padded with 1 for sub 3 elements).

A quick grep through ROCm-Developer-Tools, shows that this assumption (all 3 elements specified) is relied upon in a bunch of places: 
https://github.com/ROCm-Developer-Tools/clr/blob/38d2c56784fe2a2b9aff35822d3c9f4616189ead/rocclr/device/devkernel.cpp#L216
https://github.com/ROCm-Developer-Tools/clr/blob/38d2c56784fe2a2b9aff35822d3c9f4616189ead/rocclr/device/devkernel.cpp#L250

https://github.com/llvm/llvm-project/pull/68872


More information about the llvm-commits mailing list