[PATCH] R600/SI: Clean up checks for legality of immediate operands
Matt Arsenault
Matthew.Arsenault at amd.com
Thu Sep 11 15:23:17 PDT 2014
On 09/11/2014 06:13 PM, Tom Stellard wrote:
> On Wed, Sep 03, 2014 at 07:11:34PM -0400, Matt Arsenault wrote:
>> >
>> >On Sep 3, 2014, at 5:38 PM, Tom Stellard<thomas.stellard at amd.com> wrote:
>> >
>>> > >There are new register classes VCSrc_* which represent operands that
>>> > >can take an SGPR, VGPR or inline constant. The VSrc_* class is now used
>>> > >to represent operands that can take an SGPR, VGPR, or a 32-bit
>>> > >immediate.
>>> > >
>>> > >This allows us to have more accurate checks for legality of
>>> > >immediates, since before we had no way to distinguish between operands
>>> > >that supported any 32-bit immediate and operands which could only
>>> > >support inline constants.
> Hi Matt,
>
> Here is an updated patch.
>
> -Tom
LGTM
>
> 0001-R600-SI-Clean-up-checks-for-legality-of-immediate-op.patch
>
>
> From dfbf5c7e93d85726c7385ccb15d3649fceedcff5 Mon Sep 17 00:00:00 2001
> From: Tom Stellard<thomas.stellard at amd.com>
> Date: Mon, 18 Aug 2014 15:42:18 -0400
> Subject: [PATCH] R600/SI: Clean up checks for legality of immediate operands
>
> There are new register classes VCSrc_* which represent operands that
> can take an SGPR, VGPR or inline constant. The VSrc_* class is now used
> to represent operands that can take an SGPR, VGPR, or a 32-bit
> immediate.
>
> This allows us to have more accurate checks for legality of
> immediates, since before we had no way to distinguish between operands
> that supported any 32-bit immediate and operands which could only
> support inline constants.
> ---
> lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp | 14 +---
> lib/Target/R600/SIISelLowering.cpp | 45 +++++++++---
> lib/Target/R600/SIInstrInfo.cpp | 87 +++++++++++++++---------
> lib/Target/R600/SIInstrInfo.h | 4 ++
> lib/Target/R600/SIInstrInfo.td | 4 +-
> lib/Target/R600/SIRegisterInfo.cpp | 24 ++++++-
> lib/Target/R600/SIRegisterInfo.h | 17 +++--
> lib/Target/R600/SIRegisterInfo.td | 14 +++-
> 8 files changed, 146 insertions(+), 63 deletions(-)
>
> diff --git a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> index 65f3eb5..999fd0d 100644
> --- a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> +++ b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> @@ -85,21 +85,13 @@ MCCodeEmitter *llvm::createSIMCCodeEmitter(const MCInstrInfo &MCII,
>
> bool SIMCCodeEmitter::isSrcOperand(const MCInstrDesc &Desc,
> unsigned OpNo) const {
> - // FIXME: We need a better way to figure out which operands can be immediate
> - // values
> - //
> - // Some VOP* instructions like ADDC use VReg32 as the register class
> - // for source 0, because they read VCC and can't take an SGPR as an
> - // argument due to constant bus restrictions.
> - if (OpNo == 1 && (Desc.TSFlags & (SIInstrFlags::VOP1 | SIInstrFlags::VOP2 |
> - SIInstrFlags::VOPC)))
> - return true;
> -
> unsigned RegClass = Desc.OpInfo[OpNo].RegClass;
> return (AMDGPU::SSrc_32RegClassID == RegClass) ||
> (AMDGPU::SSrc_64RegClassID == RegClass) ||
> (AMDGPU::VSrc_32RegClassID == RegClass) ||
> - (AMDGPU::VSrc_64RegClassID == RegClass);
> + (AMDGPU::VSrc_64RegClassID == RegClass) ||
> + (AMDGPU::VCSrc_32RegClassID == RegClass) ||
> + (AMDGPU::VCSrc_64RegClassID == RegClass);
> }
>
> uint32_t SIMCCodeEmitter::getLitEncoding(const MCOperand &MO) const {
> diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
> index 629a42c..a649674 100644
> --- a/lib/Target/R600/SIISelLowering.cpp
> +++ b/lib/Target/R600/SIISelLowering.cpp
> @@ -1468,8 +1468,14 @@ SDValue SITargetLowering::PerformDAGCombine(SDNode *N,
>
> /// \brief Test if RegClass is one of the VSrc classes
> static bool isVSrc(unsigned RegClass) {
> - return AMDGPU::VSrc_32RegClassID == RegClass ||
> - AMDGPU::VSrc_64RegClassID == RegClass;
> + switch(RegClass) {
Missing space after switch
> + default: return false;
> + case AMDGPU::VSrc_32RegClassID:
> + case AMDGPU::VCSrc_32RegClassID:
> + case AMDGPU::VSrc_64RegClassID:
> + case AMDGPU::VCSrc_64RegClassID:
> + return true;
> + }
> }
>
> /// \brief Test if RegClass is one of the SSrc classes
> @@ -1581,10 +1587,9 @@ const TargetRegisterClass *SITargetLowering::getRegClassForNode(
> // If the COPY_TO_REGCLASS instruction is copying to a VSrc register
> // class, then the register class for the value could be either a
> // VReg or and SReg. In order to get a more accurate
> - if (OpClassID == AMDGPU::VSrc_32RegClassID ||
> - OpClassID == AMDGPU::VSrc_64RegClassID) {
> + if (isVSrc(OpClassID))
> return getRegClassForNode(DAG, Op.getOperand(0));
> - }
> +
> return TRI.getRegClass(OpClassID);
> case AMDGPU::EXTRACT_SUBREG: {
> int SubIdx = cast<ConstantSDNode>(Op.getOperand(1))->getZExtValue();
> @@ -1618,14 +1623,23 @@ void SITargetLowering::ensureSRegLimit(SelectionDAG &DAG, SDValue &Operand,
> unsigned RegClass,
> bool &ScalarSlotUsed) const {
>
> - // First map the operands register class to a destination class
> - if (RegClass == AMDGPU::VSrc_32RegClassID)
> - RegClass = AMDGPU::VReg_32RegClassID;
> - else if (RegClass == AMDGPU::VSrc_64RegClassID)
> - RegClass = AMDGPU::VReg_64RegClassID;
> - else
> + if (!isVSrc(RegClass))
> return;
>
> + // First map the operands register class to a destination class
> + switch (RegClass) {
> + case AMDGPU::VSrc_32RegClassID:
> + case AMDGPU::VCSrc_32RegClassID:
> + RegClass = AMDGPU::VReg_32RegClassID;
> + break;
> + case AMDGPU::VSrc_64RegClassID:
> + case AMDGPU::VCSrc_64RegClassID:
> + RegClass = AMDGPU::VReg_64RegClassID;
> + break;
> + default:
> + llvm_unreachable("Unknown vsrc reg class");
> + }
> +
> // Nothing to do if they fit naturally
> if (fitsRegClass(DAG, Operand, RegClass))
> return;
> @@ -1722,6 +1736,15 @@ SDNode *SITargetLowering::foldOperands(MachineSDNode *Node,
> // No scalar allowed when we have both VSrc and SSrc
> bool ScalarSlotUsed = HaveVSrc && HaveSSrc;
>
> + // If this instruction has an implicit use of VCC, then it can't use the
> + // constant bus.
> + for (unsigned i = 0, e = Desc->getNumImplicitUses(); i != e; ++i) {
> + if (Desc->ImplicitUses[i] == AMDGPU::VCC) {
> + ScalarSlotUsed = true;
> + break;
> + }
> + }
> +
> // Second go over the operands and try to fold them
> std::vector<SDValue> Ops;
> bool Promote2e64 = false;
> diff --git a/lib/Target/R600/SIInstrInfo.cpp b/lib/Target/R600/SIInstrInfo.cpp
> index 606cb06..93cf7ac 100644
> --- a/lib/Target/R600/SIInstrInfo.cpp
> +++ b/lib/Target/R600/SIInstrInfo.cpp
> @@ -705,7 +705,7 @@ bool SIInstrInfo::isImmOperandLegal(const MachineInstr *MI, unsigned OpNo,
> const MachineOperand &MO) const {
> const MCOperandInfo &OpInfo = get(MI->getOpcode()).OpInfo[OpNo];
>
> - assert(MO.isImm() || MO.isFPImm());
> + assert(MO.isImm() || MO.isFPImm() || MO.isTargetIndex() || MO.isFI());
>
> if (OpInfo.OperandType == MCOI::OPERAND_IMMEDIATE)
> return true;
> @@ -713,7 +713,10 @@ bool SIInstrInfo::isImmOperandLegal(const MachineInstr *MI, unsigned OpNo,
> if (OpInfo.RegClass < 0)
> return false;
>
> - return RI.regClassCanUseImmediate(OpInfo.RegClass);
> + if (isLiteralConstant(MO))
> + return RI.regClassCanUseLiteralConstant(OpInfo.RegClass);
> +
> + return RI.regClassCanUseInlineConstant(OpInfo.RegClass);
> }
>
> bool SIInstrInfo::canFoldOffset(unsigned OffsetSize, unsigned AS) {
> @@ -750,9 +753,38 @@ bool SIInstrInfo::hasModifiers(unsigned Opcode) const {
> AMDGPU::OpName::src0_modifiers) != -1;
> }
>
> +bool SIInstrInfo::usesConstantBus(const MachineRegisterInfo &MRI,
> + const MachineOperand &MO) const {
> + // Literal constants use the constant bus.
> + if (isLiteralConstant(MO))
> + return true;
> +
> + if (!MO.isReg() || !MO.isUse())
> + return false;
> +
> + if (TargetRegisterInfo::isVirtualRegister(MO.getReg()))
> + return RI.isSGPRClass(MRI.getRegClass(MO.getReg()));
> +
> +
> + // EXEC register uses the constant bus.
> + if (!MO.isImplicit() && MO.getReg() == AMDGPU::EXEC)
> + return true;
> +
> + // SGPRs use the constant bus
> + if (MO.getReg() == AMDGPU::M0 || MO.getReg() == AMDGPU::VCC ||
> + (!MO.isImplicit() &&
> + (AMDGPU::SGPR_32RegClass.contains(MO.getReg()) ||
> + AMDGPU::SGPR_64RegClass.contains(MO.getReg())))) {
> + return true;
> + }
> +
> + return false;
> +}
> +
> bool SIInstrInfo::verifyInstruction(const MachineInstr *MI,
> StringRef &ErrInfo) const {
> uint16_t Opcode = MI->getOpcode();
> + const MachineRegisterInfo &MRI = MI->getParent()->getParent()->getRegInfo();
> int Src0Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src0);
> int Src1Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src1);
> int Src2Idx = AMDGPU::getNamedOperandIdx(Opcode, AMDGPU::OpName::src2);
> @@ -769,19 +801,12 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr *MI,
> for (int i = 0, e = Desc.getNumOperands(); i != e; ++i) {
> switch (Desc.OpInfo[i].OperandType) {
> case MCOI::OPERAND_REGISTER: {
> - int RegClass = Desc.OpInfo[i].RegClass;
> - if (!RI.regClassCanUseImmediate(RegClass) &&
> - (MI->getOperand(i).isImm() || MI->getOperand(i).isFPImm())) {
> - // Handle some special cases:
> - // Src0 can of VOP1, VOP2, VOPC can be an immediate no matter what
> - // the register class.
> - if (i != Src0Idx || (!isVOP1(Opcode) && !isVOP2(Opcode) &&
> - !isVOPC(Opcode))) {
> - ErrInfo = "Expected register, but got immediate";
> + if ((MI->getOperand(i).isImm() || MI->getOperand(i).isFPImm()) &&
> + !isImmOperandLegal(MI, i, MI->getOperand(i))) {
> + ErrInfo = "Illegal immediate value for operand.";
> return false;
> }
> }
> - }
> break;
> case MCOI::OPERAND_IMMEDIATE:
> // Check if this operand is an immediate.
> @@ -821,27 +846,15 @@ bool SIInstrInfo::verifyInstruction(const MachineInstr *MI,
> unsigned SGPRUsed = AMDGPU::NoRegister;
> for (int i = 0, e = MI->getNumOperands(); i != e; ++i) {
> const MachineOperand &MO = MI->getOperand(i);
> - if (MO.isReg() && MO.isUse() &&
> - !TargetRegisterInfo::isVirtualRegister(MO.getReg())) {
> -
> - // EXEC register uses the constant bus.
> - if (!MO.isImplicit() && MO.getReg() == AMDGPU::EXEC)
> - ++ConstantBusCount;
> -
> - // SGPRs use the constant bus
> - if (MO.getReg() == AMDGPU::M0 || MO.getReg() == AMDGPU::VCC ||
> - (!MO.isImplicit() &&
> - (AMDGPU::SGPR_32RegClass.contains(MO.getReg()) ||
> - AMDGPU::SGPR_64RegClass.contains(MO.getReg())))) {
> - if (SGPRUsed != MO.getReg()) {
> + if (usesConstantBus(MRI, MO)) {
> + if (MO.isReg()) {
> + if (MO.getReg() != SGPRUsed)
> ++ConstantBusCount;
> - SGPRUsed = MO.getReg();
> - }
> + SGPRUsed = MO.getReg();
> + } else {
> + ++ConstantBusCount;
> }
> }
> - // Literal constants use the constant bus.
> - if (isLiteralConstant(MO))
> - ++ConstantBusCount;
> }
> if (ConstantBusCount > 1) {
> ErrInfo = "VOP* instruction uses the constant bus more than once";
> @@ -1090,6 +1103,18 @@ bool SIInstrInfo::isOperandLegal(const MachineInstr *MI, unsigned OpIdx,
> if (!MO)
> MO = &MI->getOperand(OpIdx);
>
> + if (usesConstantBus(MRI, *MO)) {
> + unsigned SGPRUsed = MO->isReg() ? MO->getReg() : AMDGPU::NoRegister;
> + for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
> + if (i == OpIdx)
> + continue;
> + if (usesConstantBus(MRI, MI->getOperand(i)) &&
> + MI->getOperand(i).isReg() && MI->getOperand(i).getReg() != SGPRUsed) {
> + return false;
> + }
> + }
> + }
> +
> if (MO->isReg()) {
> assert(DefinedRC);
> const TargetRegisterClass *RC = MRI.getRegClass(MO->getReg());
> @@ -1104,7 +1129,7 @@ bool SIInstrInfo::isOperandLegal(const MachineInstr *MI, unsigned OpIdx,
> // This opperand expects an immediate
> return true;
>
> - return RI.regClassCanUseImmediate(DefinedRC);
> + return isImmOperandLegal(MI, OpIdx, *MO);
> }
>
> void SIInstrInfo::legalizeOperands(MachineInstr *MI) const {
> diff --git a/lib/Target/R600/SIInstrInfo.h b/lib/Target/R600/SIInstrInfo.h
> index ed043ac..a32318a 100644
> --- a/lib/Target/R600/SIInstrInfo.h
> +++ b/lib/Target/R600/SIInstrInfo.h
> @@ -127,6 +127,10 @@ public:
> /// This function will return false if you pass it a 32-bit instruction.
> bool hasVALU32BitEncoding(unsigned Opcode) const;
>
> + /// \brief Returns true if this operand uses the constant bus.
> + bool usesConstantBus(const MachineRegisterInfo &MRI,
> + const MachineOperand &MO) const;
> +
> /// \brief Return true if this instruction has any modifiers.
> /// e.g. src[012]_mod, omod, clamp.
> bool hasModifiers(unsigned Opcode) const;
> diff --git a/lib/Target/R600/SIInstrInfo.td b/lib/Target/R600/SIInstrInfo.td
> index 2d172c3..b0a1269 100644
> --- a/lib/Target/R600/SIInstrInfo.td
> +++ b/lib/Target/R600/SIInstrInfo.td
> @@ -360,7 +360,7 @@ class getInRC32 <list<ValueType> SrcVT> {
> // Returns the register class to use for sources of VOP3 instructions for the
> // given VT.
> class getVOP3SrcForVT<ValueType VT> {
> - RegisterClass ret = !if(!eq(VT.Size, 32), VSrc_32, VSrc_64);
> + RegisterClass ret = !if(!eq(VT.Size, 32), VCSrc_32, VCSrc_64);
> }
>
> // Returns the register classes for the source arguments of a VOP3
> @@ -493,7 +493,7 @@ def VOP_F64_F64_I32 : VOPProfile <[f64, f64, i32, untyped]>;
> def VOP_I32_F32_F32 : VOPProfile <[i32, f32, f32, untyped]>;
> def VOP_I32_I32_I32 : VOPProfile <[i32, i32, i32, untyped]>;
> def VOP_I32_I32_I32_VCC : VOPProfile <[i32, i32, i32, untyped]> {
> - let Src0RC32 = VReg_32;
> + let Src0RC32 = VCSrc_32;
> }
> def VOP_I64_I64_I32 : VOPProfile <[i64, i64, i32, untyped]>;
> def VOP_I64_I64_I64 : VOPProfile <[i64, i64, i64, untyped]>;
> diff --git a/lib/Target/R600/SIRegisterInfo.cpp b/lib/Target/R600/SIRegisterInfo.cpp
> index 8663df8..cbb7a81 100644
> --- a/lib/Target/R600/SIRegisterInfo.cpp
> +++ b/lib/Target/R600/SIRegisterInfo.cpp
> @@ -252,7 +252,7 @@ unsigned SIRegisterInfo::getPhysRegSubReg(unsigned Reg,
> return SubRC->getRegister(Index + Channel);
> }
>
> -bool SIRegisterInfo::regClassCanUseImmediate(int RCID) const {
> +bool SIRegisterInfo::regClassCanUseLiteralConstant(int RCID) const {
> switch (RCID) {
> default: return false;
> case AMDGPU::SSrc_32RegClassID:
> @@ -263,11 +263,29 @@ bool SIRegisterInfo::regClassCanUseImmediate(int RCID) const {
> }
> }
>
> -bool SIRegisterInfo::regClassCanUseImmediate(
> +bool SIRegisterInfo::regClassCanUseLiteralConstant(
> const TargetRegisterClass *RC) const {
> - return regClassCanUseImmediate(RC->getID());
> + return regClassCanUseLiteralConstant(RC->getID());
> }
>
> +bool SIRegisterInfo::regClassCanUseInlineConstant(int RCID) const {
> + if (regClassCanUseLiteralConstant(RCID))
> + return true;
> +
> + switch (RCID) {
> + default: return false;
> + case AMDGPU::VCSrc_32RegClassID:
> + case AMDGPU::VCSrc_64RegClassID:
> + return true;
> + }
> +}
> +
> +bool SIRegisterInfo::regClassCanUseInlineConstant(
> + const TargetRegisterClass *RC) const {
> + return regClassCanUseInlineConstant(RC->getID());
> +}
> +
> +
> unsigned SIRegisterInfo::getPreloadedValue(const MachineFunction &MF,
> enum PreloadedValue Value) const {
>
> diff --git a/lib/Target/R600/SIRegisterInfo.h b/lib/Target/R600/SIRegisterInfo.h
> index e27e770..2439d38 100644
> --- a/lib/Target/R600/SIRegisterInfo.h
> +++ b/lib/Target/R600/SIRegisterInfo.h
> @@ -68,12 +68,21 @@ struct SIRegisterInfo : public AMDGPURegisterInfo {
> unsigned Channel) const;
>
> /// \returns True if operands defined with this register class can accept
> - /// inline immediates.
> - bool regClassCanUseImmediate(int RCID) const;
> + /// a literal constant (i.e. any 32-bit immediate).
> + bool regClassCanUseLiteralConstant(int RCID) const;
>
> /// \returns True if operands defined with this register class can accept
> - /// inline immediates.
> - bool regClassCanUseImmediate(const TargetRegisterClass *RC) const;
> + /// a literal constant (i.e. any 32-bit immediate).
> + bool regClassCanUseLiteralConstant(const TargetRegisterClass *RC) const;
> +
> + /// \returns True if operands defined with this register class can accept
> + /// an inline constant. i.e. An integer value in the range (-16, 64) or
> + /// -4.0f, -2.0f, -1.0f, -0.5f, 0.0f, 0.5f, 1.0f, 2.0f, 4.0f.
> + bool regClassCanUseInlineConstant(int RCID) const;
> +
> + /// \returns True if operands defined with this register class can accept
> + /// a literal constant. i.e. A value in the range (-16, 64).
> + bool regClassCanUseInlineConstant(const TargetRegisterClass *RC) const;
>
> enum PreloadedValue {
> TGID_X,
> diff --git a/lib/Target/R600/SIRegisterInfo.td b/lib/Target/R600/SIRegisterInfo.td
> index 8380677..24b8c8c 100644
> --- a/lib/Target/R600/SIRegisterInfo.td
> +++ b/lib/Target/R600/SIRegisterInfo.td
> @@ -200,18 +200,30 @@ def VReg_512 : RegisterClass<"AMDGPU", [v16i32, v16f32], 512, (add VGPR_512)>;
> def VReg_1 : RegisterClass<"AMDGPU", [i1], 32, (add VGPR_32)>;
>
> //===----------------------------------------------------------------------===//
> -// [SV]Src_(32|64) register classes, can have either an immediate or an register
> +// SSrc_* Operands with an SGPR or a 32-bit immediate
> //===----------------------------------------------------------------------===//
>
> def SSrc_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add SReg_32)>;
>
> def SSrc_64 : RegisterClass<"AMDGPU", [i64, f64, i1], 64, (add SReg_64)>;
>
> +//===----------------------------------------------------------------------===//
> +// VSrc_* Operands with an SGPR, VGPR or a 32-bit immediate
> +//===----------------------------------------------------------------------===//
> +
> def VSrc_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add VReg_32, SReg_32)>;
>
> def VSrc_64 : RegisterClass<"AMDGPU", [i64, f64], 64, (add VReg_64, SReg_64)>;
>
> //===----------------------------------------------------------------------===//
> +// VCSrc_* Operands with an SGPR, VGPR or an inline constant
> +//===----------------------------------------------------------------------===//
> +
> +def VCSrc_32 : RegisterClass<"AMDGPU", [i32, f32], 32, (add VReg_32, SReg_32)>;
> +
> +def VCSrc_64 : RegisterClass<"AMDGPU", [i64, f64], 64, (add VReg_64, SReg_64)>;
> +
> +//===----------------------------------------------------------------------===//
> // SGPR and VGPR register classes
> //===----------------------------------------------------------------------===//
>
> -- 1.8.5.5
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140911/54e2f70e/attachment.html>
More information about the llvm-commits
mailing list