[PATCHES] R600/SI: VI fixes
Tom Stellard
tom at stellard.net
Tue Jan 13 15:05:26 PST 2015
On Tue, Jan 13, 2015 at 05:10:32PM +0100, Marek Olšák wrote:
> Hi Tom and Matt,
>
> Attached is a new version of patch "R600/SI: Don't shrink instructions
> whose e32 encoding doesn't exist".
>
> I decided to update the hasVALU32BitEncoding function and use that
> instead. It also explains the difference between getMCOpcode returning
> -1 and (uint16_t)-1.
>
LGTM.
> Marek
>
> From 9991f2e5aa099aac44a07af75a1760cdbd0b3a42 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Marek=20Ol=C5=A1=C3=A1k?= <marek.olsak at amd.com>
> Date: Mon, 15 Dec 2014 13:09:36 +0100
> Subject: [PATCH] R600/SI: Don't shrink instructions whose e32 encoding doesn't
> exist
>
> v2: modify hasVALU32BitEncoding instead
> ---
> lib/Target/R600/AMDGPUMCInstLower.cpp | 16 +++++++---------
> lib/Target/R600/AMDGPUMCInstLower.h | 7 +++----
> lib/Target/R600/SIInstrInfo.cpp | 14 +++++++++++++-
> lib/Target/R600/SIShrinkInstructions.cpp | 9 +++++----
> 4 files changed, 28 insertions(+), 18 deletions(-)
>
> diff --git a/lib/Target/R600/AMDGPUMCInstLower.cpp b/lib/Target/R600/AMDGPUMCInstLower.cpp
> index 84ee784..3d98175 100644
> --- a/lib/Target/R600/AMDGPUMCInstLower.cpp
> +++ b/lib/Target/R600/AMDGPUMCInstLower.cpp
> @@ -40,7 +40,7 @@ AMDGPUMCInstLower::AMDGPUMCInstLower(MCContext &ctx, const AMDGPUSubtarget &st):
> { }
>
> enum AMDGPUMCInstLower::SISubtarget
> -AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) const {
> +AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) {
> switch (Gen) {
> default:
> return AMDGPUMCInstLower::SI;
> @@ -49,19 +49,17 @@ AMDGPUMCInstLower::AMDGPUSubtargetToSISubtarget(unsigned Gen) const {
> }
> }
>
> -unsigned AMDGPUMCInstLower::getMCOpcode(unsigned MIOpcode) const {
> +int AMDGPUMCInstLower::getMCOpcode(unsigned MIOpcode, unsigned Gen) {
>
> - int MCOpcode = AMDGPU::getMCOpcode(MIOpcode,
> - AMDGPUSubtargetToSISubtarget(ST.getGeneration()));
> - if (MCOpcode == -1)
> - MCOpcode = MIOpcode;
> -
> - return MCOpcode;
> + return AMDGPU::getMCOpcode(MIOpcode, AMDGPUSubtargetToSISubtarget(Gen));
> }
>
> void AMDGPUMCInstLower::lower(const MachineInstr *MI, MCInst &OutMI) const {
>
> - OutMI.setOpcode(getMCOpcode(MI->getOpcode()));
> + int MIOpcode = MI->getOpcode();
> + int MCOpcode = getMCOpcode(MIOpcode, ST.getGeneration());
> +
> + OutMI.setOpcode(MCOpcode != -1 ? MCOpcode : MIOpcode);
>
> for (const MachineOperand &MO : MI->explicit_operands()) {
> MCOperand MCOp;
> diff --git a/lib/Target/R600/AMDGPUMCInstLower.h b/lib/Target/R600/AMDGPUMCInstLower.h
> index 0ae4d11..56dc3e3 100644
> --- a/lib/Target/R600/AMDGPUMCInstLower.h
> +++ b/lib/Target/R600/AMDGPUMCInstLower.h
> @@ -31,10 +31,7 @@ class AMDGPUMCInstLower {
>
> /// Convert a member of the AMDGPUSubtarget::Generation enum to the
> /// SISubtarget enum.
> - enum SISubtarget AMDGPUSubtargetToSISubtarget(unsigned Gen) const;
> -
> - /// Get the MC opcode for this MachineInstr.
> - unsigned getMCOpcode(unsigned MIOpcode) const;
> + static enum SISubtarget AMDGPUSubtargetToSISubtarget(unsigned Gen);
>
> public:
> AMDGPUMCInstLower(MCContext &ctx, const AMDGPUSubtarget &ST);
> @@ -42,6 +39,8 @@ public:
> /// \brief Lower a MachineInstr to an MCInst
> void lower(const MachineInstr *MI, MCInst &OutMI) const;
>
> + /// Get the MC opcode for this MachineInstr.
> + static int getMCOpcode(unsigned MIOpcode, unsigned Gen);
> };
>
> } // End namespace llvm
> diff --git a/lib/Target/R600/SIInstrInfo.cpp b/lib/Target/R600/SIInstrInfo.cpp
> index 6454ad5..4105929 100644
> --- a/lib/Target/R600/SIInstrInfo.cpp
> +++ b/lib/Target/R600/SIInstrInfo.cpp
> @@ -1055,7 +1055,19 @@ bool SIInstrInfo::canFoldOffset(unsigned OffsetSize, unsigned AS) const {
> }
>
> bool SIInstrInfo::hasVALU32BitEncoding(unsigned Opcode) const {
> - return AMDGPU::getVOPe32(Opcode) != -1;
> + int Op32 = AMDGPU::getVOPe32(Opcode);
> + if (Op32 == -1)
> + return false;
> +
> + int MCOp = AMDGPU::getMCOpcode(Op32, RI.ST.getGeneration());
> +
> + // -1 means that Op32 is already a native instruction.
> + if (MCOp == -1)
> + return true;
> +
> + // (uint16_t)-1 means that Op32 is a pseudo instruction that has
> + // no encoding in the given subtarget generation.
> + return MCOp != (uint16_t)-1;
> }
>
> bool SIInstrInfo::hasModifiers(unsigned Opcode) const {
> diff --git a/lib/Target/R600/SIShrinkInstructions.cpp b/lib/Target/R600/SIShrinkInstructions.cpp
> index e97f934..e8cf57b 100644
> --- a/lib/Target/R600/SIShrinkInstructions.cpp
> +++ b/lib/Target/R600/SIShrinkInstructions.cpp
> @@ -10,6 +10,7 @@
> //
>
> #include "AMDGPU.h"
> +#include "AMDGPUMCInstLower.h"
> #include "AMDGPUSubtarget.h"
> #include "SIInstrInfo.h"
> #include "llvm/ADT/Statistic.h"
> @@ -213,13 +214,13 @@ bool SIShrinkInstructions::runOnMachineFunction(MachineFunction &MF) {
> continue;
> }
>
> - int Op32 = AMDGPU::getVOPe32(MI.getOpcode());
> -
> - // Op32 could be -1 here if we started with an instruction that had a
> + // getVOPe32 could be -1 here if we started with an instruction that had
> // a 32-bit encoding and then commuted it to an instruction that did not.
> - if (Op32 == -1)
> + if (!TII->hasVALU32BitEncoding(MI.getOpcode()))
> continue;
>
> + int Op32 = AMDGPU::getVOPe32(MI.getOpcode());
> +
> if (TII->isVOPC(Op32)) {
> unsigned DstReg = MI.getOperand(0).getReg();
> if (TargetRegisterInfo::isVirtualRegister(DstReg)) {
> --
> 2.1.0
>
More information about the llvm-commits
mailing list