From 26b2a7834e7162fd28b81c21c7c19a94baa24610 Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Fri, 26 Sep 2014 17:54:43 +0000 Subject: [PATCH] R600/SI: Fix using wrong operand indices when commuting No test since the current SIISelLowering::legalizeOperands effectively hides this, and the general uses seem to only fire on SALU instructions which don't have modifiers between the operands. When trying to use legalizeOperands immediately after instruction selection, it now sees a lot more patterns it did not see before which break on this. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@218527 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Target/R600/SIInstrInfo.cpp | 31 +++++++++++++++++--------- test/CodeGen/R600/llvm.AMDGPU.clamp.ll | 4 ++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/Target/R600/SIInstrInfo.cpp b/lib/Target/R600/SIInstrInfo.cpp index fffd698d14f..3ad448ee421 100644 --- a/lib/Target/R600/SIInstrInfo.cpp +++ b/lib/Target/R600/SIInstrInfo.cpp @@ -684,19 +684,28 @@ bool SIInstrInfo::expandPostRAPseudo(MachineBasicBlock::iterator MI) const { MachineInstr *SIInstrInfo::commuteInstruction(MachineInstr *MI, bool NewMI) const { + if (MI->getNumOperands() < 3) + return nullptr; + + int Src0Idx = AMDGPU::getNamedOperandIdx(MI->getOpcode(), + AMDGPU::OpName::src0); + assert(Src0Idx != -1 && "Should always have src0 operand"); - if (MI->getNumOperands() < 3 || !MI->getOperand(1).isReg()) + if (!MI->getOperand(Src0Idx).isReg()) return nullptr; + int Src1Idx = AMDGPU::getNamedOperandIdx(MI->getOpcode(), + AMDGPU::OpName::src1); + // Make sure it s legal to commute operands for VOP2. - if (isVOP2(MI->getOpcode()) && - (!isOperandLegal(MI, 1, &MI->getOperand(2)) || - !isOperandLegal(MI, 2, &MI->getOperand(1)))) + if ((Src1Idx != -1) && isVOP2(MI->getOpcode()) && + (!isOperandLegal(MI, Src0Idx, &MI->getOperand(Src1Idx)) || + !isOperandLegal(MI, Src1Idx, &MI->getOperand(Src0Idx)))) return nullptr; - if (!MI->getOperand(2).isReg()) { + if (Src1Idx != -1 && !MI->getOperand(Src1Idx).isReg()) { // XXX: Commute instructions with FPImm operands - if (NewMI || MI->getOperand(2).isFPImm() || + if (NewMI || MI->getOperand(Src1Idx).isFPImm() || (!isVOP2(MI->getOpcode()) && !isVOP3(MI->getOpcode()))) { return nullptr; } @@ -716,11 +725,11 @@ MachineInstr *SIInstrInfo::commuteInstruction(MachineInstr *MI, (Src2Mods && Src2Mods->getImm())) return nullptr; - unsigned Reg = MI->getOperand(1).getReg(); - unsigned SubReg = MI->getOperand(1).getSubReg(); - MI->getOperand(1).ChangeToImmediate(MI->getOperand(2).getImm()); - MI->getOperand(2).ChangeToRegister(Reg, false); - MI->getOperand(2).setSubReg(SubReg); + unsigned Reg = MI->getOperand(Src0Idx).getReg(); + unsigned SubReg = MI->getOperand(Src0Idx).getSubReg(); + MI->getOperand(Src0Idx).ChangeToImmediate(MI->getOperand(Src1Idx).getImm()); + MI->getOperand(Src1Idx).ChangeToRegister(Reg, false); + MI->getOperand(Src1Idx).setSubReg(SubReg); } else { MI = TargetInstrInfo::commuteInstruction(MI, NewMI); } diff --git a/test/CodeGen/R600/llvm.AMDGPU.clamp.ll b/test/CodeGen/R600/llvm.AMDGPU.clamp.ll index d608953a0dd..cafca080df5 100644 --- a/test/CodeGen/R600/llvm.AMDGPU.clamp.ll +++ b/test/CodeGen/R600/llvm.AMDGPU.clamp.ll @@ -6,7 +6,7 @@ declare float @llvm.AMDIL.clamp.f32(float, float, float) nounwind readnone ; FUNC-LABEL: @clamp_0_1_f32 ; SI: S_LOAD_DWORD [[ARG:s[0-9]+]], -; SI: V_ADD_F32_e64 [[RESULT:v[0-9]+]], [[ARG]], 0, 1, 0 +; SI: V_ADD_F32_e64 [[RESULT:v[0-9]+]], 0, [[ARG]], 1, 0 ; SI: BUFFER_STORE_DWORD [[RESULT]] ; SI: S_ENDPGM @@ -19,7 +19,7 @@ define void @clamp_0_1_f32(float addrspace(1)* %out, float %src) nounwind { ; FUNC-LABEL: @clamp_0_1_amdil_legacy_f32 ; SI: S_LOAD_DWORD [[ARG:s[0-9]+]], -; SI: V_ADD_F32_e64 [[RESULT:v[0-9]+]], [[ARG]], 0, 1, 0 +; SI: V_ADD_F32_e64 [[RESULT:v[0-9]+]], 0, [[ARG]], 1, 0 ; SI: BUFFER_STORE_DWORD [[RESULT]] define void @clamp_0_1_amdil_legacy_f32(float addrspace(1)* %out, float %src) nounwind { %clamp = call float @llvm.AMDIL.clamp.f32(float %src, float 0.0, float 1.0) nounwind readnone -- 2.34.1