[AMDGPU] Add asm comments if setreg changes MSBs by rampitec · Pull Request #185774 · llvm/llvm-project

rampitec · 2026-03-10T23:14:59Z

No description provided.

rampitec · 2026-03-10T23:15:00Z

[AMDGPU] Add asm comments if setreg changes MSBs #185774 👈
main

This stack of pull requests is managed by sgh.

llvmbot · 2026-03-10T23:16:56Z

@llvm/pr-subscribers-backend-amdgpu

Author: Stanislav Mekhanoshin (rampitec)

Changes

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

5 Files Affected:

(modified) llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp (+12-5)
(modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp (+19)
(modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h (+7)
(modified) llvm/test/CodeGen/AMDGPU/code-size-estimate.ll (+2-1)
(modified) llvm/test/CodeGen/AMDGPU/vgpr-setreg-mode-swar.mir (+45)

diff --git a/llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp b/llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
index fc408aa30dd87..ca8aaee450a8f 100644
--- a/llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
+++ b/llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp
@@ -436,11 +436,18 @@ void AMDGPUAsmPrinter::emitInstruction(const MachineInstr *MI) {
                              MF->getInfo<SIMachineFunctionInfo>(),
                              *OutStreamer);
 
-    if (isVerbose() && MI->getOpcode() == AMDGPU::S_SET_VGPR_MSB) {
-      unsigned V = MI->getOperand(0).getImm() & 0xff;
-      OutStreamer->AddComment(
-          " msbs: dst=" + Twine(V >> 6) + " src0=" + Twine(V & 3) +
-          " src1=" + Twine((V >> 2) & 3) + " src2=" + Twine((V >> 4) & 3));
+    if (isVerbose() && (MI->getOpcode() == AMDGPU::S_SET_VGPR_MSB ||
+                        MI->getOpcode() == AMDGPU::S_SETREG_IMM32_B32)) {
+      std::optional<unsigned> V;
+      if (MI->getOpcode() == AMDGPU::S_SETREG_IMM32_B32)
+        V = AMDGPU::convertSetRegImmToVgprMSBs(*MI,
+                                               STI.hasSetregVGPRMSBFixup());
+      else
+        V = MI->getOperand(0).getImm() & 0xff;
+      if (V.has_value())
+        OutStreamer->AddComment(
+            " msbs: dst=" + Twine(*V >> 6) + " src0=" + Twine(*V & 3) +
+            " src1=" + Twine((*V >> 2) & 3) + " src2=" + Twine((*V >> 4) & 3));
     }
 
     MCInst TmpInst;
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 710e9c5166f2e..f4ad5ba5c003a 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -3566,6 +3566,25 @@ MCRegister getVGPRWithMSBs(MCRegister Reg, unsigned MSBs,
   return RC->getRegister(Idx);
 }
 
+std::optional<unsigned> convertSetRegImmToVgprMSBs(const MachineInstr &MI,
+                                                   bool HasSetregVGPRMSBFixup) {
+  assert(MI.getOpcode() == AMDGPU::S_SETREG_IMM32_B32);
+
+  static constexpr unsigned VGPRMSBShift =
+      llvm::countr_zero_constexpr<unsigned>(AMDGPU::Hwreg::DST_VGPR_MSB);
+
+  auto [HwRegId, Offset, Size] =
+      Hwreg::HwregEncoding::decode(MI.getOperand(1).getImm());
+  if (HwRegId != Hwreg::ID_MODE ||
+      (!HasSetregVGPRMSBFixup && (Offset + Size) <= VGPRMSBShift))
+    return {};
+  unsigned Imm = MI.getOperand(0).getImm();
+  Imm = ((Imm >> Offset) & Hwreg::VGPR_MSB_MASK) >> VGPRMSBShift;
+  if (!HasSetregVGPRMSBFixup)
+    Imm &= llvm::maskTrailingOnes<unsigned>(Size);
+  return llvm::rotr<uint8_t>(static_cast<uint8_t>(Imm), /*R=*/2);
+}
+
 std::pair<const AMDGPU::OpName *, const AMDGPU::OpName *>
 getVGPRLoweringOperandTables(const MCInstrDesc &Desc) {
   static const AMDGPU::OpName VOPOps[4] = {
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index 44462e4c244bd..72c10359ebc61 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -31,6 +31,7 @@ struct Align;
 class Argument;
 class Function;
 class GlobalValue;
+class MachineInstr;
 class MCInstrInfo;
 class MCRegisterClass;
 class MCRegisterInfo;
@@ -1988,6 +1989,12 @@ unsigned getVGPREncodingMSBs(MCRegister Reg, const MCRegisterInfo &MRI);
 MCRegister getVGPRWithMSBs(MCRegister Reg, unsigned MSBs,
                            const MCRegisterInfo &MRI);
 
+/// \returns VGPR MSBs encoded in a S_SETREG_IMM32_B32 \p MI if it sets
+/// it. If \p HasSetregVGPRMSBFixup is true then size of the ID_MODE mask is
+/// ignored.
+std::optional<unsigned> convertSetRegImmToVgprMSBs(const MachineInstr &MI,
+                                                   bool HasSetregVGPRMSBFixup);
+
 // Returns a table for the opcode with a given \p Desc to map the VGPR MSB
 // set by the S_SET_VGPR_MSB to one of 4 sources. In case of VOPD returns 2
 // maps, one for X and one for Y component.
diff --git a/llvm/test/CodeGen/AMDGPU/code-size-estimate.ll b/llvm/test/CodeGen/AMDGPU/code-size-estimate.ll
index f7c6db60678f9..b59bcb7967af2 100644
--- a/llvm/test/CodeGen/AMDGPU/code-size-estimate.ll
+++ b/llvm/test/CodeGen/AMDGPU/code-size-estimate.ll
@@ -581,7 +581,8 @@ define amdgpu_ps float @s_fmaak_f32(float inreg %x, float inreg %y) {
 ;
 ; GFX1250-LABEL: s_fmaak_f32:
 ; GFX1250:       ; %bb.0:
-; GFX1250-NEXT:    s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 25, 1), 1 ; encoding: [0x41,0x06,0x80,0xb9,0x01,0x00,0x00,0x00]
+; GFX1250-NEXT:    s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 25, 1), 1 ; msbs: dst=0 src0=0 src1=0 src2=0
+; GFX1250-NEXT:    ; encoding: [0x41,0x06,0x80,0xb9,0x01,0x00,0x00,0x00]
 ; GFX1250-NEXT:    s_fmaak_f32 s0, s0, s1, 0x43800000 ; encoding: [0x00,0x01,0x80,0xa2,0x00,0x00,0x80,0x43]
 ; GFX1250-NEXT:    s_delay_alu instid0(SALU_CYCLE_3) ; encoding: [0x0b,0x00,0x87,0xbf]
 ; GFX1250-NEXT:    v_mov_b32_e32 v0, s0 ; encoding: [0x00,0x02,0x00,0x7e]
diff --git a/llvm/test/CodeGen/AMDGPU/vgpr-setreg-mode-swar.mir b/llvm/test/CodeGen/AMDGPU/vgpr-setreg-mode-swar.mir
index 422baae280f9c..fe75faec9669a 100644
--- a/llvm/test/CodeGen/AMDGPU/vgpr-setreg-mode-swar.mir
+++ b/llvm/test/CodeGen/AMDGPU/vgpr-setreg-mode-swar.mir
@@ -1,5 +1,6 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 6
 # RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1250 -run-pass=amdgpu-lower-vgpr-encoding -o - %s | FileCheck %s
+# RUN: llc -mtriple=amdgcn-amd-amdhsa -mcpu=gfx1250 -start-before=amdgpu-lower-vgpr-encoding -o - %s | FileCheck --check-prefix=ASM %s
 
 ---
 # Case 1a: Size < 12 (size=4), imm32[12:19]=0
@@ -8,6 +9,10 @@
 # vgpr256/257 (both MSB=1): S_SET_VGPR_MSB mode = (1 << 0) | (1 << 6) = 65
 # Setreg (Size=4 <= 12) resets the mode scope and clears bits[12:19] to 0.
 # No VGPR instruction follows, so bits[12:19] remain 0. Setreg imm = 5.
+
+# ASM-LABEL: {{^}}setreg_size_lt_12:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 5 ;  msbs: dst=0 src0=0 src1=0 src2=0
+
 name:            setreg_size_lt_12
 tracksRegLiveness: true
 body:             |
@@ -27,6 +32,10 @@ body:             |
 # Case 1b: Size == 12 (boundary), imm32[12:19]=0
 # Setreg (Size=12 <= 12) resets the mode scope and clears bits[12:19] to 0.
 # No VGPR instruction follows, so bits[12:19] remain 0. Setreg imm = 0xABC = 2748.
+
+# ASM-LABEL: {{^}}setreg_size_eq_12:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 12), 0xabc ;  msbs: dst=0 src0=0 src1=0 src2=0
+
 name:            setreg_size_eq_12
 tracksRegLiveness: true
 body:             |
@@ -46,6 +55,10 @@ body:             |
 # Case 1c: Size <= 12 with existing non-zero bits in imm32[12:19]
 # imm32 = 0x23005 (bits 12:19 = 0x23). Setreg resets mode scope and clears
 # bits[12:19] to 0. No VGPR instruction follows, so result = 0x00005 = 5.
+
+# ASM-LABEL: {{^}}setreg_size_lt_12_nonzero_upper:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 5 ;  msbs: dst=0 src0=0 src1=0 src2=0
+
 name:            setreg_size_lt_12_nonzero_upper
 tracksRegLiveness: true
 body:             |
@@ -66,6 +79,10 @@ body:             |
 # Case 2: Size > 12 (size=16), imm32[12:19] already matches VGPR MSBs
 # vgpr256/257: S_SET_VGPR_MSB mode = 65, MODE register mode = 5
 # imm32 = 0x5ABC = 23228 (bits 12:19 = 5), no modification needed
+
+# ASM-LABEL: {{^}}setreg_size_gt_12_match:
+# ASM: _setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 16), 0x5abc ;  msbs: dst=1 src0=1 src1=0 src2=0
+
 name:            setreg_size_gt_12_match
 tracksRegLiveness: true
 body:             |
@@ -86,6 +103,10 @@ body:             |
 # Case 3: Size > 12 (size=16), imm32[12:19] doesn't match VGPR MSBs
 # vgpr256/257: S_SET_VGPR_MSB mode = 65, MODE register mode = 5
 # imm32 = 0x23ABC = 146108 (bits 12:19 = 0x23 != 5), must insert s_set_vgpr_msb after
+
+# ASM-LABEL: {{^}}setreg_size_gt_12_mismatch:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 16), 0x23abc ;  msbs: dst=3 src0=0 src1=2 src2=0
+
 name:            setreg_size_gt_12_mismatch
 tracksRegLiveness: true
 body:             |
@@ -108,6 +129,10 @@ body:             |
 # Case 4: Non-MODE hwreg should not be modified
 # This uses ID_STATUS=2 instead of ID_MODE=1
 # vgpr256/257: S_SET_VGPR_MSB mode = 65
+
+# ASM-LABEL: {{^}}setreg_non_mode_hwreg:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_STATUS, 0, 4), 5{{$}}
+
 name:            setreg_non_mode_hwreg
 tracksRegLiveness: true
 body:             |
@@ -127,6 +152,10 @@ body:             |
 # Case 5: Size <= 12 with VGPR MSBs already present in imm32[12:19]
 # imm32 = 0x5005 = 20485 (bits 12:19 = 5). Setreg resets mode scope and clears
 # bits[12:19] to 0, regardless of prior content. Result = 5.
+
+# ASM-LABEL: {{^}}setreg_size_lt_12_already_correct:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 5 ;  msbs: dst=0 src0=0 src1=0 src2=0
+
 name:            setreg_size_lt_12_already_correct
 tracksRegLiveness: true
 body:             |
@@ -147,6 +176,10 @@ body:             |
 # Case 6: Different VGPR MSB value (using different high VGPRs)
 # vgpr512/513 (both MSB=2): S_SET_VGPR_MSB mode = (2 << 0) | (2 << 6) = 130
 # Setreg resets mode scope and clears bits[12:19] to 0. No VGPR follows. Result = 5.
+
+# ASM-LABEL: {{^}}setreg_different_vgpr_msb:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 5 ;  msbs: dst=0 src0=0 src1=0 src2=0
+
 name:            setreg_different_vgpr_msb
 tracksRegLiveness: true
 body:             |
@@ -170,6 +203,10 @@ body:             |
 #   MODE register mode = (1 << 0) | (1 << 2) | (2 << 4) = 37 (dst=1, src0=1, src1=2)
 # Piggybacking updates setreg imm32[12:19] from 0 to 37.
 # Final setreg imm = 5 | (37 << 12) = 151557
+
+# ASM-LABEL: {{^}}setreg_size_le_12_piggyback_superset:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 0x25005 ;  msbs: dst=1 src0=1 src1=2 src2=0
+
 name:            setreg_size_le_12_piggyback_superset
 tracksRegLiveness: true
 body:             |
@@ -195,6 +232,10 @@ body:             |
 # The setreg (Size=4 <= 12) resets the mode scope. Its bits[12:19] are cleared to 0.
 # The second VGPR's setMode piggybacks mode = 65 into the setreg's bits[12:19],
 # giving imm32 = 5 | (5 << 12) = 20485 = 0x5005. No separate S_SET_VGPR_MSB needed.
+
+# ASM-LABEL: {{^}}setreg_size_le_12_then_different_vgpr:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 4), 0x5005 ;  msbs: dst=1 src0=1 src1=0 src2=0
+
 name:            setreg_size_le_12_then_different_vgpr
 tracksRegLiveness: true
 body:             |
@@ -224,6 +265,10 @@ body:             |
 # Second VGPR (vgpr512/513): S_SET_VGPR_MSB mode = 130, MODE register mode = 10
 # Since MostRecentModeSet = nullptr, a new s_set_vgpr_msb is inserted.
 # New s_set_vgpr_msb imm = NewMode | (OldMode << 8) = 130 | (65 << 8) = 16770
+
+# ASM-LABEL: {{^}}setreg_size_gt_12_match_then_different_vgpr:
+# ASM: s_setreg_imm32_b32 hwreg(HW_REG_WAVE_MODE, 0, 16), 0x5abc ;  msbs: dst=1 src0=1 src1=0 src2=0
+
 name:            setreg_size_gt_12_match_then_different_vgpr
 tracksRegLiveness: true
 body:             |

rampitec · 2026-03-10T23:17:14Z

These are just comments, but the same shared code is needed for disasm to create real high VGPRs in the subsequent change. Potential verifier check shall use it too.

rampitec · 2026-03-10T23:22:37Z

    ; CHECK-NEXT: S_ENDPGM 0
    $vgpr256 = V_MOV_B32_e32 $vgpr257, implicit $exec
    ; size=4, offset=0, hwreg=MODE: simm16 = 0x1801 = 6145
    S_SETREG_IMM32_B32 5, 6145, implicit-def $mode, implicit $mode


Unrelated to this change, but I think we have a problem with setreg fixup and piggibacking. We are now relying on HW bug for correct behavior. The moment it changes we have problem. I.e., we rely on setreg to update the mode when the mask does not cover MSBs. I think we either need to update previous S_SET_VGPR_MSB as well if possible, extend the mask of S_SETREG_IMM32_B32 to cover MSBs if possible, or just emit yet another S_SET_VGPR_MSB if not. Otherwise the moment HW bug is fixed we have problem.

Or maybe revert the piggibacking at all.

It is controlled by a target feature, so if the issue is resolved later, as the first step, we can just remove the target feature.

Except that we never do because nobody tells us it is fixed.

¯_(ツ)_/¯

shiltian · 2026-03-11T11:34:46Z

    ; CHECK-NEXT: S_ENDPGM 0
    $vgpr256 = V_MOV_B32_e32 $vgpr257, implicit $exec
    ; size=4, offset=0, hwreg=MODE: simm16 = 0x1801 = 6145
    S_SETREG_IMM32_B32 5, 6145, implicit-def $mode, implicit $mode


¯_(ツ)_/¯

Regenerate checks after two recent commits that caused extra stuff to be added at the end of assembly lines, so the existing checks did not fail. - llvm#179414 added "nv" to loads and stores on GFX1250. - llvm#185774 added "msbs" comments on setreg instructions.

…#187325) Regenerate checks after two recent commits that caused extra stuff to be added at the end of assembly lines, so the existing checks did not fail. - #179414 added "nv" to loads and stores on GFX1250. - #185774 added "msbs" comments on setreg instructions.

rampitec requested review from jayfoad and shiltian March 10, 2026 23:16

rampitec marked this pull request as ready for review March 10, 2026 23:16

llvmbot added the backend:AMDGPU label Mar 10, 2026

rampitec commented Mar 10, 2026

View reviewed changes

rampitec force-pushed the users/rampitec/setreg-msb-comments branch from 4911812 to 69a5eb9 Compare March 10, 2026 23:41

shiltian reviewed Mar 11, 2026

View reviewed changes

Comment thread llvm/lib/Target/AMDGPU/AMDGPUMCInstLower.cpp

shiltian approved these changes Mar 11, 2026

View reviewed changes

[AMDGPU] Add asm comments if setreg changes MSBs

bdd9bcf

rampitec force-pushed the users/rampitec/setreg-msb-comments branch from 69a5eb9 to bdd9bcf Compare March 11, 2026 17:47

rampitec merged commit f16d872 into main Mar 11, 2026
10 checks passed

rampitec deleted the users/rampitec/setreg-msb-comments branch March 11, 2026 18:37

jayfoad mentioned this pull request Mar 18, 2026

[AMDGPU] Regenerate codegen tests to check extra stuff at end of line #187325

Merged

ayasin-a pushed a commit to ayasin-a/llvm-project that referenced this pull request Apr 19, 2026

[AMDGPU] Add asm comments if setreg changes MSBs (llvm#185774)

1ca5045

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[AMDGPU] Add asm comments if setreg changes MSBs - #185774

[AMDGPU] Add asm comments if setreg changes MSBs#185774
rampitec merged 1 commit into
mainfrom
users/rampitec/setreg-msb-comments

rampitec commented Mar 10, 2026

Uh oh!

rampitec commented Mar 10, 2026 •

edited

Loading

Uh oh!

llvmbot commented Mar 10, 2026

Uh oh!

rampitec commented Mar 10, 2026 •

edited

Loading

Uh oh!

rampitec Mar 10, 2026

Uh oh!

rampitec Mar 11, 2026

Uh oh!

shiltian Mar 11, 2026

Uh oh!

rampitec Mar 11, 2026

Uh oh!

shiltian Mar 11, 2026

Uh oh!

Uh oh!

Uh oh!

shiltian Mar 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rampitec commented Mar 10, 2026

Uh oh!

rampitec commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 10, 2026

Uh oh!

rampitec commented Mar 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rampitec Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

rampitec Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

shiltian Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

rampitec Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

shiltian Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shiltian Mar 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rampitec commented Mar 10, 2026 •

edited

Loading

rampitec commented Mar 10, 2026 •

edited

Loading