[llvm] [AMDGPU] Refactor isDPALU_DPP to only check if the instruction requires the feature (PR #224373)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Sep 17 10:47:36 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-amdgpu
Author: Domenic Nutile (saxlungs)
<details>
<summary>Changes</summary>
Previously the helper function was a combination of checking if the instruction required the feature and if the feature is available. This behavior diverges from similar isDPALU_DPP32BitOpc and can cause confusion. This refactor aligns the two for consistency, and users should also check if the feature is available if needed
---
Full diff: https://github.com/llvm/llvm-project/pull/224373.diff
6 Files Affected:
- (modified) llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp (+1)
- (modified) llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp (+11-12)
- (modified) llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp (+1)
- (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+1)
- (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp (+4-6)
- (modified) llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h (+1-5)
``````````diff
diff --git a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
index 15560e0758a19..623678c454b12 100644
--- a/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
+++ b/llvm/lib/Target/AMDGPU/AsmParser/AMDGPUAsmParser.cpp
@@ -5151,6 +5151,7 @@ bool AMDGPUAsmParser::validateDPP(const MCInst &Inst,
unsigned DppCtrl = Inst.getOperand(DppCtrlIdx).getImm();
if (!AMDGPU::isLegalDPALU_DPPControl(getSTI(), DppCtrl) &&
+ getSTI().hasFeature(AMDGPU::FeatureDPALU_DPP) &&
AMDGPU::isDPALU_DPP(MII.get(Opc), MII, getSTI())) {
// DP ALU DPP is supported for row_newbcast only on GFX9* and row_share
// only on GFX12.
diff --git a/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp b/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
index 4fb9abb707040..dac86c1ecf7d9 100644
--- a/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
+++ b/llvm/lib/Target/AMDGPU/GCNDPPCombine.cpp
@@ -769,19 +769,18 @@ bool GCNDPPCombine::combineDPPMov(MachineInstr &MovMI) const {
// Without DPALU DPP there are no 64-bit DPP encodings. The 64-bit move is
// rejected above, but a 32-bit move folded into a source of a 64-bit
// instruction reaches here, so the operands have to be checked too.
- if (!ST->hasFeature(AMDGPU::FeatureDPALU_DPP) &&
- (AMDGPU::isDPALU_DPP32BitOpc(OrigOp) ||
- AMDGPU::hasAny64BitVGPROperands(TII->get(OrigOp), *TII, *ST))) {
- LLVM_DEBUG(dbgs() << " " << OrigMI
- << " failed: DPP ALU DPP is not supported\n");
- break;
- }
+ if (AMDGPU::isDPALU_DPP(TII->get(OrigOp), *TII, *ST)) {
+ if (!ST->hasFeature(AMDGPU::FeatureDPALU_DPP)) {
+ LLVM_DEBUG(dbgs() << " " << OrigMI
+ << " failed: DPP ALU DPP is not supported\n");
+ break;
+ }
- if (!AMDGPU::isLegalDPALU_DPPControl(*ST, DppCtrlVal) &&
- AMDGPU::isDPALU_DPP(TII->get(OrigOp), *TII, *ST)) {
- LLVM_DEBUG(dbgs() << " " << OrigMI
- << " failed: not valid 64-bit DPP control value\n");
- break;
+ if (!AMDGPU::isLegalDPALU_DPPControl(*ST, DppCtrlVal)) {
+ LLVM_DEBUG(dbgs() << " " << OrigMI
+ << " failed: not valid 64-bit DPP control value\n");
+ break;
+ }
}
LLVM_DEBUG(dbgs() << " combining: " << OrigMI);
diff --git a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
index 12880e6692918..0aece53db1eb0 100644
--- a/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
+++ b/llvm/lib/Target/AMDGPU/MCTargetDesc/AMDGPUInstPrinter.cpp
@@ -1105,6 +1105,7 @@ void AMDGPUInstPrinter::printDPPCtrl(const MCInst *MI, unsigned OpNo,
const MCInstrDesc &Desc = MII.get(MI->getOpcode());
if (!AMDGPU::isLegalDPALU_DPPControl(STI, Imm) &&
+ STI.hasFeature(AMDGPU::FeatureDPALU_DPP) &&
AMDGPU::isDPALU_DPP(Desc, MII, STI)) {
O << " /* DP ALU dpp only supports "
<< (isGFX12(STI) ? "row_share" : "row_newbcast") << " */";
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 2c014218914b0..aae4675791a7d 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6047,6 +6047,7 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr &MI,
if (Opcode != AMDGPU::V_MOV_B64_DPP_PSEUDO &&
!AMDGPU::isLegalDPALU_DPPControl(ST, DC) &&
+ ST.hasFeature(AMDGPU::FeatureDPALU_DPP) &&
AMDGPU::isDPALU_DPP(Desc, *this, ST)) {
ErrInfo = "Invalid dpp_ctrl value: "
"DP ALU dpp only support row_newbcast";
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
index 11b4dbc24c085..45a5a1f582d2f 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.cpp
@@ -3616,8 +3616,9 @@ bool supportsScaleOffset(const MCInstrInfo &MII, unsigned Opcode) {
return false;
}
-bool hasAny64BitVGPROperands(const MCInstrDesc &OpDesc, const MCInstrInfo &MII,
- const MCSubtargetInfo &ST) {
+static bool hasAny64BitVGPROperands(const MCInstrDesc &OpDesc,
+ const MCInstrInfo &MII,
+ const MCSubtargetInfo &ST) {
for (auto OpName : {OpName::vdst, OpName::src0, OpName::src1, OpName::src2}) {
int Idx = getNamedOperandIdx(OpDesc.getOpcode(), OpName);
if (Idx == -1)
@@ -3656,11 +3657,8 @@ bool isDPALU_DPP32BitOpc(unsigned Opc) {
bool isDPALU_DPP(const MCInstrDesc &OpDesc, const MCInstrInfo &MII,
const MCSubtargetInfo &ST) {
- if (!ST.hasFeature(AMDGPU::FeatureDPALU_DPP))
- return false;
-
if (isDPALU_DPP32BitOpc(OpDesc.getOpcode()))
- return ST.hasFeature(AMDGPU::FeatureGFX1250Insts);
+ return true;
return hasAny64BitVGPROperands(OpDesc, MII, ST);
}
diff --git a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
index e124073f0466e..99d62739f06ba 100644
--- a/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
+++ b/llvm/lib/Target/AMDGPU/Utils/AMDGPUBaseInfo.h
@@ -1761,14 +1761,10 @@ inline bool isLegalDPALU_DPPControl(const MCSubtargetInfo &ST, unsigned DC) {
return false;
}
-/// \returns true if an instruction may have a 64-bit VGPR operand.
-bool hasAny64BitVGPROperands(const MCInstrDesc &OpDesc, const MCInstrInfo &MII,
- const MCSubtargetInfo &ST);
-
/// \returns true if an instruction is a DP ALU DPP without any 64-bit operands.
bool isDPALU_DPP32BitOpc(unsigned Opc);
-/// \returns true if an instruction is a DP ALU DPP.
+/// \returns true if an instruction is a DP ALU DPP
bool isDPALU_DPP(const MCInstrDesc &OpDesc, const MCInstrInfo &MII,
const MCSubtargetInfo &ST);
``````````
</details>
https://github.com/llvm/llvm-project/pull/224373
More information about the llvm-commits
mailing list