[PATCHES] R600/SI: VI fixes

[Tom Stellard]

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
>