[llvm] [AMDGPU] Fix SDWA selection combining (PR #218512)
Arseniy Obolenskiy via llvm-commits
llvm-commits at lists.llvm.org
Tue Sep 15 01:48:32 PDT 2026
https://github.com/aobolensk updated https://github.com/llvm/llvm-project/pull/218512
>From 005f8476848522ec647b0a34ac9b4a1ad32df8f3 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Mon, 24 Aug 2026 22:21:38 +0200
Subject: [PATCH 1/5] [AMDGPU] Fix SDWA selection composition for high fields
The operand already moved its field to the low bits, so a high selection on top of it reads only extension bits, even when the two selections match
---
llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp | 41 ++++++++++++----
.../sdwa-peephole-instr-combine-sel-dst.mir | 2 +
.../sdwa-peephole-instr-combine-sel-src.mir | 47 ++++++++++++++-----
3 files changed, 67 insertions(+), 23 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
index f8a28983d3e11..7d24d492686a2 100644
--- a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
@@ -163,8 +163,8 @@ class SDWASrcOperand : public SDWAOperand {
bool getNeg() const { return Neg; }
bool getSext() const { return Sext; }
- uint64_t getSrcMods(const SIInstrInfo *TII,
- const MachineOperand *SrcOp) const;
+ uint64_t getSrcMods(const SIInstrInfo *TII, const MachineOperand *SrcOp,
+ SdwaSel ExistingSel) const;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream& OS) const override;
@@ -187,6 +187,7 @@ class SDWADstOperand : public SDWAOperand {
bool convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) override;
bool canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) override;
+ virtual void applyDstSel(MachineOperand &DstSelOp) const;
SdwaSel getDstSel() const { return DstSel; }
DstUnused getDstUnused() const { return DstUn; }
@@ -209,6 +210,7 @@ class SDWADstPreserveOperand : public SDWADstOperand {
bool convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) override;
bool canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) override;
+ void applyDstSel(MachineOperand &DstSelOp) const override;
MachineOperand *getPreservedOperand() const { return Preserve; }
@@ -322,7 +324,7 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
if (Sel == SdwaSel::DWORD)
return OperandSel;
- if (Sel == OperandSel || OperandSel == SdwaSel::DWORD)
+ if (OperandSel == SdwaSel::DWORD)
return Sel;
if (Sel == SdwaSel::WORD_1 || Sel == SdwaSel::BYTE_2 ||
@@ -341,11 +343,15 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
return SdwaSel::WORD_1;
}
+ if (Sel == SdwaSel::BYTE_0 && OperandSel == SdwaSel::BYTE_0)
+ return SdwaSel::BYTE_0;
+
return {};
}
uint64_t SDWASrcOperand::getSrcMods(const SIInstrInfo *TII,
- const MachineOperand *SrcOp) const {
+ const MachineOperand *SrcOp,
+ SdwaSel ExistingSel) const {
uint64_t Mods = 0;
const auto *MI = SrcOp->getParent();
if (TII->getNamedOperand(*MI, AMDGPU::OpName::src0) == SrcOp) {
@@ -362,8 +368,8 @@ uint64_t SDWASrcOperand::getSrcMods(const SIInstrInfo *TII,
"Float and integer src modifiers can't be set simultaneously");
Mods |= Abs ? SISrcMods::ABS : 0u;
Mods ^= Neg ? SISrcMods::NEG : 0u;
- } else if (Sext) {
- Mods |= SISrcMods::SEXT;
+ } else if (ExistingSel == SdwaSel::DWORD) {
+ Mods = (Mods & ~uint64_t(SISrcMods::SEXT)) | (Sext ? SISrcMods::SEXT : 0u);
}
return Mods;
@@ -504,7 +510,7 @@ bool SDWASrcOperand::convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) {
if (!IsPreserveSrc) {
SdwaSel ExistingSel = static_cast<SdwaSel>(SrcSel->getImm());
SrcSel->setImm(*combineSdwaSel(ExistingSel, getSrcSel()));
- SrcMods->setImm(getSrcMods(TII, Src));
+ SrcMods->setImm(getSrcMods(TII, Src, ExistingSel));
}
getTargetOperand()->setIsKill(false);
return true;
@@ -593,8 +599,7 @@ bool SDWADstOperand::convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) {
MachineOperand *DstSel= TII->getNamedOperand(MI, AMDGPU::OpName::dst_sel);
assert(DstSel);
- SdwaSel ExistingSel = static_cast<SdwaSel>(DstSel->getImm());
- DstSel->setImm(combineSdwaSel(ExistingSel, getDstSel()).value());
+ applyDstSel(*DstSel);
MachineOperand *DstUnused= TII->getNamedOperand(MI, AMDGPU::OpName::dst_unused);
assert(DstUnused);
@@ -614,6 +619,11 @@ bool SDWADstOperand::canCombineSelections(const MachineInstr &MI,
return canCombineOpSel(MI, TII, AMDGPU::OpName::dst_sel, getDstSel());
}
+void SDWADstOperand::applyDstSel(MachineOperand &DstSelOp) const {
+ SdwaSel ExistingSel = static_cast<SdwaSel>(DstSelOp.getImm());
+ DstSelOp.setImm(*combineSdwaSel(ExistingSel, getDstSel()));
+}
+
bool SDWADstPreserveOperand::convertToSDWA(MachineInstr &MI,
const SIInstrInfo *TII) {
// MI should be moved right before v_or_b32.
@@ -645,7 +655,18 @@ bool SDWADstPreserveOperand::convertToSDWA(MachineInstr &MI,
bool SDWADstPreserveOperand::canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) {
- return SDWADstOperand::canCombineSelections(MI, TII);
+ if (!TII->isSDWA(MI.getOpcode()))
+ return true;
+
+ // DstSel was captured from the dst_sel of MI, so there is nothing to compose.
+ SdwaSel ExistingSel = static_cast<SdwaSel>(
+ TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel));
+ return ExistingSel == getDstSel();
+}
+
+// MI is re-emitted with the dst_sel this operand was matched with.
+void SDWADstPreserveOperand::applyDstSel(MachineOperand &DstSelOp) const {
+ assert(static_cast<SdwaSel>(DstSelOp.getImm()) == getDstSel());
}
std::optional<int64_t>
diff --git a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
index 1d4fb1c68ce6b..ab50ec64cf8b4 100644
--- a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
+++ b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
@@ -39,6 +39,7 @@ body: |
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 5, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHLREV_B32_e32_:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_e32 16, [[V_LSHRREV_B32_sdwa]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 6, 6, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
@@ -232,6 +233,7 @@ body: |
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 3, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHLREV_B32_e32_:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_e32 24, [[V_LSHRREV_B32_sdwa]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 6, 6, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
diff --git a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
index f138b6e730dde..8ca69c98e84c9 100644
--- a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
+++ b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
@@ -440,7 +440,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B16_e32_:%[0-9]+]]:vgpr_32 = V_LSHRREV_B16_e32 8, [[V_LSHRREV_B32_sdwa]], implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 1, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B16_e32_]], 0, 1, 0, 6, 1, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -572,7 +572,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 16, 8, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 2, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_BFE_I32_e64_]], 0, 1, 0, 6, 2, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -704,7 +704,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 24, 8, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 3, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_BFE_I32_e64_]], 0, 1, 0, 6, 3, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -814,7 +814,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 16, 16, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 5, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_BFE_I32_e64_]], 0, 1, 0, 6, 5, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -836,7 +836,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 16, 16, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 5, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 5, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -902,7 +902,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 16, 16, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 3, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 3, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -924,7 +924,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 16, 16, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 2, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 2, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -968,7 +968,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 5, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 5, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -990,7 +990,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 4, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 4, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -1012,7 +1012,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 3, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 3, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -1034,7 +1034,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 2, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 2, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -1056,7 +1056,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 1, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 1, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -1078,7 +1078,7 @@ body: |
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 5, 0, implicit $exec
; CHECK-NEXT: [[V_BFE_I32_e64_:%[0-9]+]]:vgpr_32 = V_BFE_I32_e64 [[V_LSHRREV_B32_sdwa]], 0, 32, implicit $exec
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 16, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[V_LSHRREV_B32_sdwa]], 0, [[V_LSHRREV_B32_sdwa]], 0, 1, 0, 6, 0, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
%2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %1, 0, 1, 0, 5, 0, implicit $exec
@@ -1087,3 +1087,24 @@ body: |
S_ENDPGM 0
...
+
+---
+# A stale sext must not survive narrowing the selection to a zero-extended WORD_0.
+name: op_select_word_0_instr_stale_sext
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0
+ ; CHECK-LABEL: name: op_select_word_0_instr_stale_sext
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+ ; CHECK-NEXT: [[V_AND_B32_e32_:%[0-9]+]]:vgpr_32 = V_AND_B32_e32 65535, [[COPY]], implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 6, 4, implicit $exec
+ ; CHECK-NEXT: S_ENDPGM 0
+ %1:vgpr_32 = COPY $vgpr0
+ %2:vgpr_32 = V_AND_B32_e32 65535, %1, implicit $exec
+ %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 16, %2, 0, 1, 0, 6, 6, implicit $exec
+
+ S_ENDPGM 0
+...
>From 41bd8f6b9bfb06feb274c25ba99fd3c16e259cb7 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Tue, 1 Sep 2026 15:42:42 +0200
Subject: [PATCH 2/5] Address comments
---
llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp | 60 ++++++-------------
.../sdwa-peephole-instr-combine-sel-dst.mir | 16 ++---
2 files changed, 27 insertions(+), 49 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
index 7d24d492686a2..5b6470536f620 100644
--- a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
@@ -187,7 +187,6 @@ class SDWADstOperand : public SDWAOperand {
bool convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) override;
bool canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) override;
- virtual void applyDstSel(MachineOperand &DstSelOp) const;
SdwaSel getDstSel() const { return DstSel; }
DstUnused getDstUnused() const { return DstUn; }
@@ -210,7 +209,6 @@ class SDWADstPreserveOperand : public SDWADstOperand {
bool convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) override;
bool canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) override;
- void applyDstSel(MachineOperand &DstSelOp) const override;
MachineOperand *getPreservedOperand() const { return Preserve; }
@@ -314,10 +312,11 @@ static MachineOperand *findSingleRegDef(const MachineOperand *Reg,
return MRI->getOneDef(Reg->getReg());
}
-/// Combine an SDWA instruction's existing SDWA selection \p Sel with
+/// Combine an SDWA instruction's existing source selection \p Sel with
/// the SDWA selection \p OperandSel of its operand. If the selections
/// are compatible, return the combined selection, otherwise return a
-/// nullopt.
+/// nullopt. Destination selections are never composed, see
+/// SDWADstOperand::canCombineSelections.
/// For example, if we have Sel = BYTE_0 Sel and OperandSel = WORD_1:
/// BYTE_0 Sel (WORD_1 Sel (%X)) -> BYTE_2 Sel (%X)
static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
@@ -331,7 +330,9 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
Sel == SdwaSel::BYTE_3)
return {};
- if (OperandSel == SdwaSel::WORD_0)
+ // OperandSel selects a field that wholly contains the one Sel selects.
+ if (OperandSel == SdwaSel::WORD_0 ||
+ (OperandSel == SdwaSel::BYTE_0 && Sel == SdwaSel::BYTE_0))
return Sel;
if (OperandSel == SdwaSel::WORD_1) {
@@ -343,9 +344,6 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
return SdwaSel::WORD_1;
}
- if (Sel == SdwaSel::BYTE_0 && OperandSel == SdwaSel::BYTE_0)
- return SdwaSel::BYTE_0;
-
return {};
}
@@ -369,6 +367,7 @@ uint64_t SDWASrcOperand::getSrcMods(const SIInstrInfo *TII,
Mods |= Abs ? SISrcMods::ABS : 0u;
Mods ^= Neg ? SISrcMods::NEG : 0u;
} else if (ExistingSel == SdwaSel::DWORD) {
+ // A narrower selection already fixed the field, so drop any stale SEXT.
Mods = (Mods & ~uint64_t(SISrcMods::SEXT)) | (Sext ? SISrcMods::SEXT : 0u);
}
@@ -516,18 +515,6 @@ bool SDWASrcOperand::convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) {
return true;
}
-/// Verify that the SDWA selection operand \p SrcSelOpName of the SDWA
-/// instruction \p MI can be combined with the selection \p OpSel.
-static bool canCombineOpSel(const MachineInstr &MI, const SIInstrInfo *TII,
- AMDGPU::OpName SrcSelOpName, SdwaSel OpSel) {
- assert(TII->isSDWA(MI.getOpcode()));
-
- const MachineOperand *SrcSelOp = TII->getNamedOperand(MI, SrcSelOpName);
- SdwaSel SrcSel = static_cast<SdwaSel>(SrcSelOp->getImm());
-
- return combineSdwaSel(SrcSel, OpSel).has_value();
-}
-
/// Verify that \p Op is the same register as the operand of the SDWA
/// instruction \p MI named by \p SrcOpName and that the SDWA
/// selection \p SrcSelOpName can be combined with the \p OpSel.
@@ -541,7 +528,9 @@ static bool canCombineOpSel(const MachineInstr &MI, const SIInstrInfo *TII,
if (!Src || !isSameReg(*Src, *Op))
return true;
- return canCombineOpSel(MI, TII, SrcSelOpName, OpSel);
+ SdwaSel SrcSel =
+ static_cast<SdwaSel>(TII->getNamedOperand(MI, SrcSelOpName)->getImm());
+ return combineSdwaSel(SrcSel, OpSel).has_value();
}
bool SDWASrcOperand::canCombineSelections(const MachineInstr &MI,
@@ -599,7 +588,7 @@ bool SDWADstOperand::convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) {
MachineOperand *DstSel= TII->getNamedOperand(MI, AMDGPU::OpName::dst_sel);
assert(DstSel);
- applyDstSel(*DstSel);
+ DstSel->setImm(getDstSel());
MachineOperand *DstUnused= TII->getNamedOperand(MI, AMDGPU::OpName::dst_unused);
assert(DstUnused);
@@ -616,12 +605,8 @@ bool SDWADstOperand::canCombineSelections(const MachineInstr &MI,
if (!TII->isSDWA(MI.getOpcode()))
return true;
- return canCombineOpSel(MI, TII, AMDGPU::OpName::dst_sel, getDstSel());
-}
-
-void SDWADstOperand::applyDstSel(MachineOperand &DstSelOp) const {
- SdwaSel ExistingSel = static_cast<SdwaSel>(DstSelOp.getImm());
- DstSelOp.setImm(*combineSdwaSel(ExistingSel, getDstSel()));
+ // Composing dst_sel also depends on dst_unused, so require none set yet.
+ return TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel) == SdwaSel::DWORD;
}
bool SDWADstPreserveOperand::convertToSDWA(MachineInstr &MI,
@@ -655,18 +640,10 @@ bool SDWADstPreserveOperand::convertToSDWA(MachineInstr &MI,
bool SDWADstPreserveOperand::canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) {
- if (!TII->isSDWA(MI.getOpcode()))
- return true;
-
- // DstSel was captured from the dst_sel of MI, so there is nothing to compose.
- SdwaSel ExistingSel = static_cast<SdwaSel>(
- TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel));
- return ExistingSel == getDstSel();
-}
-
-// MI is re-emitted with the dst_sel this operand was matched with.
-void SDWADstPreserveOperand::applyDstSel(MachineOperand &DstSelOp) const {
- assert(static_cast<SdwaSel>(DstSelOp.getImm()) == getDstSel());
+ // DstSel came from the dst_sel already on MI, only dst_unused changes here.
+ assert(!TII->isSDWA(MI.getOpcode()) ||
+ TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel) == getDstSel());
+ return true;
}
std::optional<int64_t>
@@ -1200,7 +1177,8 @@ bool isConvertibleToSDWA(MachineInstr &MI,
return false;
// Check if target supports this SDWA opcode
- if (TII->pseudoToMCOpcode(Opc) == -1)
+ if (TII->pseudoToMCOpcode(Opc) == -1 ||
+ TII->pseudoToMCOpcode(AMDGPU::getSDWAOp(Opc)) == -1)
return false;
if (MachineOperand *Src0 = TII->getNamedOperand(MI, AMDGPU::OpName::src0)) {
diff --git a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
index ab50ec64cf8b4..b8fedf31e32a0 100644
--- a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
+++ b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-dst.mir
@@ -1,11 +1,8 @@
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
# RUN: llc -mtriple=amdgpu10.30 -simplify-mir -run-pass=si-peephole-sdwa -o - %s | FileCheck %s
-# Test the combination of SDWA selections in si-peephole-sdwa. In each
-# example, the SDWA destination selection specified on the first instruction
-# must be combined with the destination selection that the pass determines
-# for the operand, i.e. the second instruction. In the cases where
-# this is not possible, no conversion should occur.
+# Destination selections are never composed, so conversion happens only when
+# the first instruction still selects DWORD.
---
name: op_select_word_1_instr_select_dword
@@ -60,7 +57,8 @@ body: |
; CHECK: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 5, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 4, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHLREV_B32_e32_:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_e32 16, [[V_LSHRREV_B32_sdwa]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 6, 6, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
@@ -125,7 +123,8 @@ body: |
; CHECK: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 3, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHLREV_B32_e32_:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_e32 16, [[V_LSHRREV_B32_sdwa]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 6, 6, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
@@ -146,7 +145,8 @@ body: |
; CHECK: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
- ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 0, 0, 0, 0, implicit $exec
+ ; CHECK-NEXT: [[V_LSHLREV_B32_e32_:%[0-9]+]]:vgpr_32 = V_LSHLREV_B32_e32 16, [[V_LSHRREV_B32_sdwa]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa1:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 2, 0, 6, 6, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
%1:vgpr_32 = COPY $vgpr0
>From f8608b049edab281f48401ca17fd6a0cb3f44de5 Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Mon, 7 Sep 2026 23:21:54 +0200
Subject: [PATCH 3/5] Address comemnts
---
llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp | 60 +++++++++----------
.../sdwa-peephole-instr-combine-sel-src.mir | 51 +++++++++++++++-
2 files changed, 75 insertions(+), 36 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
index 2ad168f0a11da..02eeb059490e6 100644
--- a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
@@ -160,8 +160,7 @@ class SDWASrcOperand : public SDWAOperand {
bool getNeg() const { return Neg; }
bool getSext() const { return Sext; }
- uint64_t getSrcMods(const SIInstrInfo *TII, const MachineOperand *SrcOp,
- SdwaSel ExistingSel) const;
+ uint64_t getSrcMods(uint64_t Mods, SdwaSel ExistingSel) const;
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
void print(raw_ostream& OS) const override;
@@ -309,14 +308,15 @@ static MachineOperand *findSingleRegDef(const MachineOperand *Reg,
return MRI->getOneDef(Reg->getReg());
}
-/// Combine an SDWA instruction's existing source selection \p Sel with
-/// the SDWA selection \p OperandSel of its operand. If the selections
+/// Combine an SDWA source instruction's existing source selection \p Sel
+/// with the SDWA selection \p OperandSel of its operand. If the selections
/// are compatible, return the combined selection, otherwise return a
-/// nullopt. Destination selections are never composed, see
+/// nullopt. Destination selections are never combined this way, see
/// SDWADstOperand::canCombineSelections.
/// For example, if we have Sel = BYTE_0 Sel and OperandSel = WORD_1:
/// BYTE_0 Sel (WORD_1 Sel (%X)) -> BYTE_2 Sel (%X)
-static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
+static std::optional<SdwaSel> combineSdwaSrcSel(SdwaSel Sel,
+ SdwaSel OperandSel) {
if (Sel == SdwaSel::DWORD)
return OperandSel;
@@ -327,11 +327,14 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
Sel == SdwaSel::BYTE_3)
return {};
- // OperandSel selects a field that wholly contains the one Sel selects.
- if (OperandSel == SdwaSel::WORD_0 ||
- (OperandSel == SdwaSel::BYTE_0 && Sel == SdwaSel::BYTE_0))
+ // WORD_0 wholly contains the BYTE_0, BYTE_1 and WORD_0 that Sel can be here.
+ if (OperandSel == SdwaSel::WORD_0)
return Sel;
+ // Reading BYTE_0 of a BYTE_0 field is that same byte.
+ if (OperandSel == SdwaSel::BYTE_0 && Sel == SdwaSel::BYTE_0)
+ return SdwaSel::BYTE_0;
+
if (OperandSel == SdwaSel::WORD_1) {
if (Sel == SdwaSel::BYTE_0)
return SdwaSel::BYTE_2;
@@ -344,28 +347,20 @@ static std::optional<SdwaSel> combineSdwaSel(SdwaSel Sel, SdwaSel OperandSel) {
return {};
}
-uint64_t SDWASrcOperand::getSrcMods(const SIInstrInfo *TII,
- const MachineOperand *SrcOp,
- SdwaSel ExistingSel) const {
- uint64_t Mods = 0;
- const auto *MI = SrcOp->getParent();
- if (TII->getNamedOperand(*MI, AMDGPU::OpName::src0) == SrcOp) {
- if (auto *Mod = TII->getNamedOperand(*MI, AMDGPU::OpName::src0_modifiers)) {
- Mods = Mod->getImm();
- }
- } else if (TII->getNamedOperand(*MI, AMDGPU::OpName::src1) == SrcOp) {
- if (auto *Mod = TII->getNamedOperand(*MI, AMDGPU::OpName::src1_modifiers)) {
- Mods = Mod->getImm();
- }
- }
+uint64_t SDWASrcOperand::getSrcMods(uint64_t Mods, SdwaSel ExistingSel) const {
if (Abs || Neg) {
assert(!Sext &&
"Float and integer src modifiers can't be set simultaneously");
Mods |= Abs ? SISrcMods::ABS : 0u;
Mods ^= Neg ? SISrcMods::NEG : 0u;
} else if (ExistingSel == SdwaSel::DWORD) {
- // A narrower selection already fixed the field, so drop any stale SEXT.
- Mods = (Mods & ~uint64_t(SISrcMods::SEXT)) | (Sext ? SISrcMods::SEXT : 0u);
+ // SEXT is a no-op for a DWORD selection, so MI may carry one that means
+ // nothing. The combined selection is the operand's field, so use the
+ // operand's SEXT and drop whatever MI had. When ExistingSel narrows the
+ // field instead, the operand's SEXT bits fall outside the combined
+ // field, so MI's own SEXT is kept as is.
+ Mods &= ~uint64_t(SISrcMods::SEXT);
+ Mods |= Sext ? SISrcMods::SEXT : 0u;
}
return Mods;
@@ -505,8 +500,8 @@ bool SDWASrcOperand::convertToSDWA(MachineInstr &MI, const SIInstrInfo *TII) {
copyRegOperand(*Src, *getTargetOperand());
if (!IsPreserveSrc) {
SdwaSel ExistingSel = static_cast<SdwaSel>(SrcSel->getImm());
- SrcSel->setImm(*combineSdwaSel(ExistingSel, getSrcSel()));
- SrcMods->setImm(getSrcMods(TII, Src, ExistingSel));
+ SrcSel->setImm(*combineSdwaSrcSel(ExistingSel, getSrcSel()));
+ SrcMods->setImm(getSrcMods(SrcMods->getImm(), ExistingSel));
}
getTargetOperand()->setIsKill(false);
return true;
@@ -525,9 +520,8 @@ static bool canCombineOpSel(const MachineInstr &MI, const SIInstrInfo *TII,
if (!Src || !isSameReg(*Src, *Op))
return true;
- SdwaSel SrcSel =
- static_cast<SdwaSel>(TII->getNamedOperand(MI, SrcSelOpName)->getImm());
- return combineSdwaSel(SrcSel, OpSel).has_value();
+ auto SrcSel = static_cast<SdwaSel>(TII->getNamedImmOperand(MI, SrcSelOpName));
+ return combineSdwaSrcSel(SrcSel, OpSel).has_value();
}
bool SDWASrcOperand::canCombineSelections(const MachineInstr &MI,
@@ -602,7 +596,8 @@ bool SDWADstOperand::canCombineSelections(const MachineInstr &MI,
if (!TII->isSDWA(MI.getOpcode()))
return true;
- // Composing dst_sel also depends on dst_unused, so require none set yet.
+ // Destination selections are never combined, so only fold into an MI that
+ // is not yet writing a sub-field of its destination.
return TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel) == SdwaSel::DWORD;
}
@@ -638,8 +633,7 @@ bool SDWADstPreserveOperand::convertToSDWA(MachineInstr &MI,
bool SDWADstPreserveOperand::canCombineSelections(const MachineInstr &MI,
const SIInstrInfo *TII) {
// DstSel came from the dst_sel already on MI, only dst_unused changes here.
- assert(!TII->isSDWA(MI.getOpcode()) ||
- TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel) == getDstSel());
+ assert(TII->getNamedImmOperand(MI, AMDGPU::OpName::dst_sel) == getDstSel());
return true;
}
diff --git a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
index 98997f5709a7b..903c38f2ee34b 100644
--- a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
+++ b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
@@ -991,13 +991,14 @@ body: |
...
---
-# A stale sext must not survive narrowing the selection to a zero-extended WORD_0.
-name: op_select_word_0_instr_stale_sext
+# The sext on the instruction is a no-op for its DWORD selection. Combining
+# the selection with the operand's WORD_0 must not make the sext take effect.
+name: op_select_word_0_instr_dword_sext
tracksRegLiveness: true
body: |
bb.0:
liveins: $vgpr0
- ; CHECK-LABEL: name: op_select_word_0_instr_stale_sext
+ ; CHECK-LABEL: name: op_select_word_0_instr_dword_sext
; CHECK: liveins: $vgpr0
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
@@ -1010,3 +1011,47 @@ body: |
S_ENDPGM 0
...
+
+---
+# The operand already moved BYTE_1 down into BYTE_0, so the BYTE_1 selection of
+# the instruction reads different bits than BYTE_1 of the operand's source. The
+# two selections must not be combined even though they are equal.
+name: op_select_byte_1_instr_sdwa_select_byte_1
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0
+ ; CHECK-LABEL: name: op_select_byte_1_instr_sdwa_select_byte_1
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+ ; CHECK-NEXT: [[V_LSHRREV_B16_e32_:%[0-9]+]]:vgpr_32 = V_LSHRREV_B16_e32 8, [[COPY]], implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[V_LSHRREV_B16_e32_]], 0, 1, 0, 6, 1, implicit $exec
+ ; CHECK-NEXT: S_ENDPGM 0
+ %1:vgpr_32 = COPY $vgpr0
+ %2:vgpr_32 = V_LSHRREV_B16_e32 8, %1, implicit $exec
+ %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %2, 0, 1, 0, 6, 1, implicit $exec
+
+ S_ENDPGM 0
+...
+
+---
+# BYTE_0 of BYTE_0 is BYTE_0, so equal selections do combine in this case.
+name: op_select_byte_0_instr_sdwa_select_byte_0
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: $vgpr0
+ ; CHECK-LABEL: name: op_select_byte_0_instr_sdwa_select_byte_0
+ ; CHECK: liveins: $vgpr0
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:vgpr_32 = COPY $vgpr0
+ ; CHECK-NEXT: [[V_AND_B32_e32_:%[0-9]+]]:vgpr_32 = V_AND_B32_e32 255, [[COPY]], implicit $exec
+ ; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 6, 0, implicit $exec
+ ; CHECK-NEXT: S_ENDPGM 0
+ %1:vgpr_32 = COPY $vgpr0
+ %2:vgpr_32 = V_AND_B32_e32 255, %1, implicit $exec
+ %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %2, 0, 1, 0, 6, 0, implicit $exec
+
+ S_ENDPGM 0
+...
>From a82cefaf5146aed06b099197caa5adfdeb5cbebe Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Mon, 14 Sep 2026 18:36:27 +0200
Subject: [PATCH 4/5] Compact register numbers
---
.../sdwa-peephole-instr-combine-sel-src.mir | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
index 903c38f2ee34b..a6ce448a41011 100644
--- a/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
+++ b/llvm/test/CodeGen/AMDGPU/sdwa-peephole-instr-combine-sel-src.mir
@@ -1005,9 +1005,9 @@ body: |
; CHECK-NEXT: [[V_AND_B32_e32_:%[0-9]+]]:vgpr_32 = V_AND_B32_e32 65535, [[COPY]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 6, 4, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
- %1:vgpr_32 = COPY $vgpr0
- %2:vgpr_32 = V_AND_B32_e32 65535, %1, implicit $exec
- %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 16, %2, 0, 1, 0, 6, 6, implicit $exec
+ %0:vgpr_32 = COPY $vgpr0
+ %1:vgpr_32 = V_AND_B32_e32 65535, %0, implicit $exec
+ %2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0, 16, %1, 0, 1, 0, 6, 6, implicit $exec
S_ENDPGM 0
...
@@ -1028,9 +1028,9 @@ body: |
; CHECK-NEXT: [[V_LSHRREV_B16_e32_:%[0-9]+]]:vgpr_32 = V_LSHRREV_B16_e32 8, [[COPY]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[V_LSHRREV_B16_e32_]], 0, 1, 0, 6, 1, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
- %1:vgpr_32 = COPY $vgpr0
- %2:vgpr_32 = V_LSHRREV_B16_e32 8, %1, implicit $exec
- %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %2, 0, 1, 0, 6, 1, implicit $exec
+ %0:vgpr_32 = COPY $vgpr0
+ %1:vgpr_32 = V_LSHRREV_B16_e32 8, %0, implicit $exec
+ %2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0, 0, %1, 0, 1, 0, 6, 1, implicit $exec
S_ENDPGM 0
...
@@ -1049,9 +1049,9 @@ body: |
; CHECK-NEXT: [[V_AND_B32_e32_:%[0-9]+]]:vgpr_32 = V_AND_B32_e32 255, [[COPY]], implicit $exec
; CHECK-NEXT: [[V_LSHRREV_B32_sdwa:%[0-9]+]]:vgpr_32 = V_LSHRREV_B32_sdwa 0, [[COPY]], 0, [[COPY]], 0, 1, 0, 6, 0, implicit $exec
; CHECK-NEXT: S_ENDPGM 0
- %1:vgpr_32 = COPY $vgpr0
- %2:vgpr_32 = V_AND_B32_e32 255, %1, implicit $exec
- %3:vgpr_32 = V_LSHRREV_B32_sdwa 0, %1, 0, %2, 0, 1, 0, 6, 0, implicit $exec
+ %0:vgpr_32 = COPY $vgpr0
+ %1:vgpr_32 = V_AND_B32_e32 255, %0, implicit $exec
+ %2:vgpr_32 = V_LSHRREV_B32_sdwa 0, %0, 0, %1, 0, 1, 0, 6, 0, implicit $exec
S_ENDPGM 0
...
>From cbc65313abe78925d5593b61c3555a5a52285f7c Mon Sep 17 00:00:00 2001
From: Arseniy Obolenskiy <arseniy.obolenskiy at amd.com>
Date: Tue, 15 Sep 2026 10:48:18 +0200
Subject: [PATCH 5/5] comment
---
llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
index 02eeb059490e6..6181b4f695d21 100644
--- a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
@@ -311,8 +311,11 @@ static MachineOperand *findSingleRegDef(const MachineOperand *Reg,
/// Combine an SDWA source instruction's existing source selection \p Sel
/// with the SDWA selection \p OperandSel of its operand. If the selections
/// are compatible, return the combined selection, otherwise return a
-/// nullopt. Destination selections are never combined this way, see
-/// SDWADstOperand::canCombineSelections.
+/// nullopt.
+/// This applies to src_sel only. Extracting from an already extracted field
+/// is again a single field, but dst_sel writes a field and leaves the rest to
+/// dst_unused, so two of them do not fold into one.
+/// See SDWADstOperand::canCombineSelections.
/// For example, if we have Sel = BYTE_0 Sel and OperandSel = WORD_1:
/// BYTE_0 Sel (WORD_1 Sel (%X)) -> BYTE_2 Sel (%X)
static std::optional<SdwaSel> combineSdwaSrcSel(SdwaSel Sel,
More information about the llvm-commits
mailing list