[AMDGPU] Print workgroup_processor_mode metadata field as a boolean

[lucas-rami]

The workgroup_processor_mode metadata field is a boolean according to the documentation. We currently serialize its value as "0" (false) or "1" (true) when these should be literal "false" or "true", in line with other boolean metadata fields. This fixes the AMDGPU metadata streamer and verifier to resolve this mismatch.

Fixes SWDEV-524612.

[llvmbot]

@llvm/pr-subscribers-llvm-binary-utilities

@llvm/pr-subscribers-backend-amdgpu

Author: Lucas Ramirez (lucas-rami)

Changes

The workgroup_processor_mode metadata field is a boolean according to the documentation. We currently serialize its value as "0" (false) or "1" (true) when these should be literal "false" or "true", in line with other boolean metadata fields. This fixes the AMDGPU metadata streamer and verifier to resolve this mismatch.

Fixes SWDEV-524612.

---

Full diff: https://github.com/llvm/llvm-project/pull/135627.diff

4 Files Affected:

• (modified) llvm/lib/BinaryFormat/AMDGPUMetadataVerifier.cpp (+2-1)
• (modified) llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp (+1-1)
• (modified) llvm/test/CodeGen/AMDGPU/hsa-generic-target-features.ll (+2-2)
• (modified) llvm/test/CodeGen/AMDGPU/hsa-metadata-workgroup-processor-mode-v5.ll (+2-2)

diff --git a/llvm/lib/BinaryFormat/AMDGPUMetadataVerifier.cpp b/llvm/lib/BinaryFormat/AMDGPUMetadataVerifier.cpp
index 33eed07c46292..b7497665b64e7 100644
--- a/llvm/lib/BinaryFormat/AMDGPUMetadataVerifier.cpp
+++ b/llvm/lib/BinaryFormat/AMDGPUMetadataVerifier.cpp
@@ -262,7 +262,8 @@ bool MetadataVerifier::verifyKernel(msgpack::DocNode &Node) {
   if (!verifyScalarEntry(KernelMap, ".uses_dynamic_stack", false,
                          msgpack::Type::Boolean))
     return false;
-  if (!verifyIntegerEntry(KernelMap, ".workgroup_processor_mode", false))
+  if (!verifyScalarEntry(KernelMap, ".workgroup_processor_mode", false,
+                         msgpack::Type::Boolean))
     return false;
   if (!verifyIntegerEntry(KernelMap, ".kernarg_segment_align", true))
     return false;
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp b/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
index 2991778a1bbc7..bc8a126ae0132 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUHSAMetadataStreamer.cpp
@@ -510,7 +510,7 @@ MetadataStreamerMsgPackV4::getHSAKernelProps(const MachineFunction &MF,
 
   if (CodeObjectVersion >= AMDGPU::AMDHSA_COV5 && STM.supportsWGP())
     Kern[".workgroup_processor_mode"] =
-        Kern.getDocument()->getNode(ProgramInfo.WgpMode);
+        Kern.getDocument()->getNode((bool)ProgramInfo.WgpMode);
 
   // FIXME: The metadata treats the minimum as 16?
   Kern[".kernarg_segment_align"] =
diff --git a/llvm/test/CodeGen/AMDGPU/hsa-generic-target-features.ll b/llvm/test/CodeGen/AMDGPU/hsa-generic-target-features.ll
index 1801082b60030..f401b0443e1c0 100644
--- a/llvm/test/CodeGen/AMDGPU/hsa-generic-target-features.ll
+++ b/llvm/test/CodeGen/AMDGPU/hsa-generic-target-features.ll
@@ -17,9 +17,9 @@
 ; Checks 10.1, 10.3 and 11 generic targets allow cumode/wave64.
 
 ; NOCU:    .amdhsa_workgroup_processor_mode 0
-; NOCU:    .workgroup_processor_mode: 0
+; NOCU:    .workgroup_processor_mode: false
 ; CU:      .amdhsa_workgroup_processor_mode 1
-; CU:      .workgroup_processor_mode: 1
+; CU:      .workgroup_processor_mode: true
 
 ; W64:      .amdhsa_wavefront_size32 0
 ; W32:      .amdhsa_wavefront_size32 1
diff --git a/llvm/test/CodeGen/AMDGPU/hsa-metadata-workgroup-processor-mode-v5.ll b/llvm/test/CodeGen/AMDGPU/hsa-metadata-workgroup-processor-mode-v5.ll
index e6c3fe139ffbb..a42249993d47f 100644
--- a/llvm/test/CodeGen/AMDGPU/hsa-metadata-workgroup-processor-mode-v5.ll
+++ b/llvm/test/CodeGen/AMDGPU/hsa-metadata-workgroup-processor-mode-v5.ll
@@ -4,9 +4,9 @@
 ; RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1100 < %s | FileCheck -check-prefix=GFX10-CU %s
 
 ; GFX10:    .amdhsa_workgroup_processor_mode 0
-; GFX10:    .workgroup_processor_mode: 0
+; GFX10:    .workgroup_processor_mode: false
 ; GFX10-CU: .amdhsa_workgroup_processor_mode 1
-; GFX10-CU: .workgroup_processor_mode: 1
+; GFX10-CU: .workgroup_processor_mode: true
 
 define amdgpu_kernel void @wavefrontsize() {
 entry:

[shiltian]

LGTM, though I'm not sure what kind of difference it can make. Can you request a full testing cycle internally to test whether the runtime can fully support it?

[Pierre-vh]

Why is the explicit cast needed here? Does it really make a difference?

[lucas-rami]

The cast makes this a call to Document::getNode(bool) instead of Document::getNode(int), which returns a DocNode with the Type::Boolean type instead of the Type::Int type. That gets printed as false/true instead of 0/1.

[Pierre-vh]

Shouldn't WgpMode be defined as a boolean in SIProgramInfo directly?

[lucas-rami]

Yes I think so too, but all other boolean metadata fields are also stored as uint32_t in SIProgramInfo. I thought perhaps there was an underlying reason for this I did not see, in any case I did not want to create an exception for this field. If there is really no reason these can all be changed to bool in a future PR.

[arsenm]

probably should just change this, I don't see why we would want this to be a particular type

[shiltian]

TBH I think whoever consumes this should be able to handle 0/false and 1/true.