[Mesa-dev] [PATCH 2/2] R600/SI: simplify and fix SMRD encoding

Tom Stellard tom at stellard.net
Tue Feb 5 06:45:43 PST 2013


On Mon, Feb 04, 2013 at 05:54:14PM +0100, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
> 
> The _SGPR variants where wrong.
>

*were* wrong.

This looks good, it's nice to see so much of the c++ code disapear.

Reviewed-by: Tom Stellard <thomas.stellard at amd.com>

> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
>  lib/Target/R600/AMDGPUCodeEmitter.h                |    4 -
>  lib/Target/R600/AMDILISelDAGToDAG.cpp              |   53 ------------
>  lib/Target/R600/MCTargetDesc/AMDGPUMCCodeEmitter.h |    4 -
>  lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp   |   34 --------
>  lib/Target/R600/SIInstrInfo.td                     |   90 ++++++++------------
>  lib/Target/R600/SIInstructions.td                  |   39 ++++++++-
>  6 files changed, 70 insertions(+), 154 deletions(-)
> 
> diff --git a/lib/Target/R600/AMDGPUCodeEmitter.h b/lib/Target/R600/AMDGPUCodeEmitter.h
> index 84f3588..5d61cd0 100644
> --- a/lib/Target/R600/AMDGPUCodeEmitter.h
> +++ b/lib/Target/R600/AMDGPUCodeEmitter.h
> @@ -38,10 +38,6 @@ public:
>                                      unsigned OpNo) const {
>      return 0;
>    }
> -  virtual uint32_t SMRDmemriEncode(const MachineInstr &MI, unsigned OpNo)
> -                                                                   const {
> -    return 0;
> -  }
>  };
>  
>  } // End namespace llvm
> diff --git a/lib/Target/R600/AMDILISelDAGToDAG.cpp b/lib/Target/R600/AMDILISelDAGToDAG.cpp
> index 2699409..a88e8c7 100644
> --- a/lib/Target/R600/AMDILISelDAGToDAG.cpp
> +++ b/lib/Target/R600/AMDILISelDAGToDAG.cpp
> @@ -72,8 +72,6 @@ private:
>    bool SelectGlobalValueConstantOffset(SDValue Addr, SDValue& IntPtr);
>    bool SelectGlobalValueVariableOffset(SDValue Addr,
>        SDValue &BaseReg, SDValue& Offset);
> -  bool SelectADDR8BitOffset(SDValue Addr, SDValue& Base, SDValue& Offset);
> -  bool SelectADDRReg(SDValue Addr, SDValue& Base, SDValue& Offset);
>    bool SelectADDRVTX_READ(SDValue Addr, SDValue &Base, SDValue &Offset);
>    bool SelectADDRIndirect(SDValue Addr, SDValue &Base, SDValue &Offset);
>  
> @@ -527,43 +525,6 @@ bool AMDGPUDAGToDAGISel::SelectGlobalValueVariableOffset(SDValue Addr,
>    return false;
>  }
>  
> -bool AMDGPUDAGToDAGISel::SelectADDR8BitOffset(SDValue Addr, SDValue& Base,
> -                                             SDValue& Offset) {
> -  if (Addr.getOpcode() == ISD::TargetExternalSymbol ||
> -      Addr.getOpcode() == ISD::TargetGlobalAddress) {
> -    return false;
> -  }
> -
> -
> -  if (Addr.getOpcode() == ISD::ADD) {
> -    bool Match = false;
> -
> -    // Find the base ptr and the offset
> -    for (unsigned i = 0; i < Addr.getNumOperands(); i++) {
> -      SDValue Arg = Addr.getOperand(i);
> -      ConstantSDNode * OffsetNode = dyn_cast<ConstantSDNode>(Arg);
> -      // This arg isn't a constant so it must be the base PTR.
> -      if (!OffsetNode) {
> -        Base = Addr.getOperand(i);
> -        continue;
> -      }
> -      // Check if the constant argument fits in 8-bits.  The offset is in bytes
> -      // so we need to convert it to dwords.
> -      if (isUInt<8>(OffsetNode->getZExtValue() >> 2)) {
> -        Match = true;
> -        Offset = CurDAG->getTargetConstant(OffsetNode->getZExtValue() >> 2,
> -                                           MVT::i32);
> -      }
> -    }
> -    return Match;
> -  }
> -
> -  // Default case, no offset
> -  Base = Addr;
> -  Offset = CurDAG->getTargetConstant(0, MVT::i32);
> -  return true;
> -}
> -
>  bool AMDGPUDAGToDAGISel::SelectADDRVTX_READ(SDValue Addr, SDValue &Base,
>                                             SDValue &Offset) {
>    ConstantSDNode * IMMOffset;
> @@ -591,20 +552,6 @@ bool AMDGPUDAGToDAGISel::SelectADDRVTX_READ(SDValue Addr, SDValue &Base,
>    return true;
>  }
>  
> -bool AMDGPUDAGToDAGISel::SelectADDRReg(SDValue Addr, SDValue& Base,
> -                                      SDValue& Offset) {
> -  if (Addr.getOpcode() == ISD::TargetExternalSymbol ||
> -      Addr.getOpcode() == ISD::TargetGlobalAddress  ||
> -      Addr.getOpcode() != ISD::ADD) {
> -    return false;
> -  }
> -
> -  Base = Addr.getOperand(0);
> -  Offset = Addr.getOperand(1);
> -
> -  return true;
> -}
> -
>  bool AMDGPUDAGToDAGISel::SelectADDRIndirect(SDValue Addr, SDValue &Base,
>                                              SDValue &Offset) {
>    ConstantSDNode *C;
> diff --git a/lib/Target/R600/MCTargetDesc/AMDGPUMCCodeEmitter.h b/lib/Target/R600/MCTargetDesc/AMDGPUMCCodeEmitter.h
> index 9d0d6cf..3b3816a 100644
> --- a/lib/Target/R600/MCTargetDesc/AMDGPUMCCodeEmitter.h
> +++ b/lib/Target/R600/MCTargetDesc/AMDGPUMCCodeEmitter.h
> @@ -49,10 +49,6 @@ public:
>                                     SmallVectorImpl<MCFixup> &Fixups) const {
>      return 0;
>    }
> -  virtual uint32_t SMRDmemriEncode(const MCInst &MI, unsigned OpNo,
> -                                   SmallVectorImpl<MCFixup> &Fixups) const {
> -    return 0;
> -  }
>  };
>  
>  } // End namespace llvm
> diff --git a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> index c47dc99..7bf89f5 100644
> --- a/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> +++ b/lib/Target/R600/MCTargetDesc/SIMCCodeEmitter.cpp
> @@ -92,10 +92,6 @@ public:
>    virtual unsigned GPR4AlignEncode(const MCInst &MI, unsigned OpNo,
>                                     SmallVectorImpl<MCFixup> &Fixup) const;
>  
> -  /// \brief Encoding for SMRD indexed loads
> -  virtual uint32_t SMRDmemriEncode(const MCInst &MI, unsigned OpNo,
> -                                   SmallVectorImpl<MCFixup> &Fixup) const;
> -
>    /// \brief Post-Encoder method for VOP instructions
>    virtual uint64_t VOPPostEncode(const MCInst &MI, uint64_t Value) const;
>  
> @@ -183,36 +179,6 @@ unsigned SIMCCodeEmitter::GPR4AlignEncode(const MCInst &MI,
>    return GPRAlign(MI, OpNo, 2);
>  }
>  
> -#define SMRD_OFFSET_MASK 0xff
> -#define SMRD_IMM_SHIFT 8
> -#define SMRD_SBASE_MASK 0x3f
> -#define SMRD_SBASE_SHIFT 9
> -/// This function is responsibe for encoding the offset
> -/// and the base ptr for SMRD instructions it should return a bit string in
> -/// this format:
> -///
> -/// OFFSET = bits{7-0}
> -/// IMM    = bits{8}
> -/// SBASE  = bits{14-9}
> -///
> -uint32_t SIMCCodeEmitter::SMRDmemriEncode(const MCInst &MI, unsigned OpNo,
> -                                        SmallVectorImpl<MCFixup> &Fixup) const {
> -  uint32_t Encoding;
> -
> -  const MCOperand &OffsetOp = MI.getOperand(OpNo + 1);
> -
> -  //XXX: Use this function for SMRD loads with register offsets
> -  assert(OffsetOp.isImm());
> -
> -  Encoding =
> -      (getMachineOpValue(MI, OffsetOp, Fixup) & SMRD_OFFSET_MASK)
> -    | (1 << SMRD_IMM_SHIFT) //XXX If the Offset is a register we shouldn't set this bit
> -    | ((GPR2AlignEncode(MI, OpNo, Fixup) & SMRD_SBASE_MASK) << SMRD_SBASE_SHIFT)
> -    ;
> -
> -  return Encoding;
> -}
> -
>  //===----------------------------------------------------------------------===//
>  // Post Encoder Callbacks
>  //===----------------------------------------------------------------------===//
> diff --git a/lib/Target/R600/SIInstrInfo.td b/lib/Target/R600/SIInstrInfo.td
> index baa7956..80b4a9b 100644
> --- a/lib/Target/R600/SIInstrInfo.td
> +++ b/lib/Target/R600/SIInstrInfo.td
> @@ -38,6 +38,11 @@ def SIvcc_bitcast : SDNode<"SIISD::VCC_BITCAST",
>    SDTypeProfile<1, 1, [SDTCisInt<0>, SDTCisInt<1>]>
>  >;
>  
> +// SMRD takes a 64bit memory address and can only add an 32bit offset
> +def SIadd64bit32bit : SDNode<"ISD::ADD",
> +  SDTypeProfile<1, 2, [SDTCisSameAs<0, 1>, SDTCisVT<0, i64>, SDTCisVT<2, i32>]>
> +>;
> +
>  // Transformation function, extract the lower 32bit of a 64bit immediate
>  def LO32 : SDNodeXForm<imm, [{
>    return CurDAG->getTargetConstant(N->getZExtValue() & 0xffffffff, MVT::i32);
> @@ -48,6 +53,20 @@ def HI32 : SDNodeXForm<imm, [{
>    return CurDAG->getTargetConstant(N->getZExtValue() >> 32, MVT::i32);
>  }]>;
>  
> +def IMM8bitDWORD : ImmLeaf <
> +  i32, [{
> +    return (Imm & ~0x3FC) == 0;
> +  }], SDNodeXForm<imm, [{
> +    return CurDAG->getTargetConstant(
> +      N->getZExtValue() >> 2, MVT::i32);
> +  }]>
> +>;
> +
> +def IMM12bit : ImmLeaf <
> +  i16,
> +  [{return isUInt<12>(Imm);}]
> +>;
> +
>  class InstSI <dag outs, dag ins, string asm, list<dag> pattern> :
>      AMDGPUInst<outs, ins, asm, pattern> {
>  
> @@ -79,49 +98,16 @@ class SIOperand <ValueType vt, dag opInfo>: Operand <vt> {
>    let MIOperandInfo = opInfo;
>  }
>  
> -def IMM16bit : ImmLeaf <
> -  i16,
> -  [{return isInt<16>(Imm);}]
> ->;
> -
> -def IMM8bit : ImmLeaf <
> -  i32,
> -  [{return (int32_t)Imm >= 0 && (int32_t)Imm <= 0xff;}]
> ->;
> -
> -def IMM12bit : ImmLeaf <
> -  i16,
> -  [{return (int16_t)Imm >= 0 && (int16_t)Imm <= 0xfff;}]
> ->;
> -
> -def IMM32bitIn64bit : ImmLeaf <
> -  i64,
> -  [{return isInt<32>(Imm);}]
> ->;
> -
>  class GPR4Align <RegisterClass rc> : Operand <vAny> {
>    let EncoderMethod = "GPR4AlignEncode";
>    let MIOperandInfo = (ops rc:$reg); 
>  }
>  
> -class GPR2Align <RegisterClass rc, ValueType vt> : Operand <vt> {
> +class GPR2Align <RegisterClass rc> : Operand <iPTR> {
>    let EncoderMethod = "GPR2AlignEncode";
>    let MIOperandInfo = (ops rc:$reg);
>  }
>  
> -def SMRDmemrr : Operand<iPTR> {
> -  let MIOperandInfo = (ops SReg_64, SReg_32);
> -  let EncoderMethod = "GPR2AlignEncode";
> -}
> -
> -def SMRDmemri : Operand<iPTR> {
> -  let MIOperandInfo = (ops SReg_64, i32imm);
> -  let EncoderMethod = "SMRDmemriEncode";
> -}
> -
> -def ADDR_Reg     : ComplexPattern<i64, 2, "SelectADDRReg", [], []>;
> -def ADDR_Offset8 : ComplexPattern<i64, 2, "SelectADDR8BitOffset", [], []>;
> -
>  let Uses = [EXEC] in {
>  
>  def EXP : Enc64<
> @@ -272,17 +258,15 @@ class MUBUF <bits<7> op, dag outs, dag ins, string asm, list<dag> pattern> :
>  
>  } // End Uses = [EXEC]
>  
> -class SMRD <bits<5> op, dag outs, dag ins, string asm, list<dag> pattern> :
> -    Enc32<outs, ins, asm, pattern> {
> +class SMRD <bits<5> op, bits<1> imm, dag outs, dag ins, string asm,
> +            list<dag> pattern> : Enc32<outs, ins, asm, pattern> {
>  
>    bits<7> SDST;
> -  bits<15> PTR;
> -  bits<8> OFFSET = PTR{7-0};
> -  bits<1> IMM    = PTR{8};
> -  bits<6> SBASE  = PTR{14-9};
> +  bits<6> SBASE;
> +  bits<8> OFFSET;
>    
>    let Inst{7-0} = OFFSET;
> -  let Inst{8} = IMM;
> +  let Inst{8} = imm;
>    let Inst{14-9} = SBASE;
>    let Inst{21-15} = SDST;
>    let Inst{26-22} = op;
> @@ -573,29 +557,23 @@ class MTBUF_Store_Helper <bits<3> op, string asm, RegisterClass regClass> : MTBU
>    let mayLoad = 0;
>  }
>  
> -multiclass SMRD_Helper <bits<5> op, string asm, RegisterClass dstClass,
> -                        ValueType vt> {
> +multiclass SMRD_Helper <bits<5> op, string asm, RegisterClass dstClass> {
>    def _IMM : SMRD <
> -              op,
> -              (outs dstClass:$dst),
> -              (ins SMRDmemri:$src0),
> -              asm,
> -              [(set (vt dstClass:$dst), (constant_load ADDR_Offset8:$src0))]
> +             op, 1,
> +             (outs dstClass:$dst),
> +             (ins GPR2Align<SReg_64>:$sbase, i32imm:$offset),
> +             asm,
> +             []
>    >;
>  
>    def _SGPR : SMRD <
> -              op,
> +              op, 0,
>                (outs dstClass:$dst),
> -              (ins SMRDmemrr:$src0),
> +              (ins GPR2Align<SReg_64>:$sbase, SReg_32:$soff),
>                asm,
> -              [(set (vt dstClass:$dst), (constant_load ADDR_Reg:$src0))]
> +              []
>    >;
>  }
>  
> -multiclass SMRD_32 <bits<5> op, string asm, RegisterClass dstClass> {
> -  defm _F32 : SMRD_Helper <op, asm, dstClass, f32>;
> -  defm _I32 : SMRD_Helper <op, asm, dstClass, i32>;
> -}
> -
>  include "SIInstrFormats.td"
>  include "SIInstructions.td"
> diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td
> index f3238fc..865bf1f 100644
> --- a/lib/Target/R600/SIInstructions.td
> +++ b/lib/Target/R600/SIInstructions.td
> @@ -461,11 +461,13 @@ def TBUFFER_LOAD_FORMAT_XYZW : MTBUF_Load_Helper <0x00000003, "TBUFFER_LOAD_FORM
>  //def TBUFFER_STORE_FORMAT_XYZ : MTBUF_ <0x00000006, "TBUFFER_STORE_FORMAT_XYZ", []>;
>  //def TBUFFER_STORE_FORMAT_XYZW : MTBUF_ <0x00000007, "TBUFFER_STORE_FORMAT_XYZW", []>;
>  
> -defm S_LOAD_DWORD : SMRD_32 <0x00000000, "S_LOAD_DWORD", SReg_32>;
> +let mayLoad = 1 in {
> +
> +defm S_LOAD_DWORD : SMRD_Helper <0x00000000, "S_LOAD_DWORD", SReg_32>;
>  
>  //def S_LOAD_DWORDX2 : SMRD_DWORDX2 <0x00000001, "S_LOAD_DWORDX2", []>;
> -defm S_LOAD_DWORDX4 : SMRD_Helper <0x00000002, "S_LOAD_DWORDX4", SReg_128, v4i32>;
> -defm S_LOAD_DWORDX8 : SMRD_Helper <0x00000003, "S_LOAD_DWORDX8", SReg_256, v8i32>;
> +defm S_LOAD_DWORDX4 : SMRD_Helper <0x00000002, "S_LOAD_DWORDX4", SReg_128>;
> +defm S_LOAD_DWORDX8 : SMRD_Helper <0x00000003, "S_LOAD_DWORDX8", SReg_256>;
>  //def S_LOAD_DWORDX16 : SMRD_DWORDX16 <0x00000004, "S_LOAD_DWORDX16", []>;
>  //def S_BUFFER_LOAD_DWORD : SMRD_ <0x00000008, "S_BUFFER_LOAD_DWORD", []>;
>  //def S_BUFFER_LOAD_DWORDX2 : SMRD_DWORDX2 <0x00000009, "S_BUFFER_LOAD_DWORDX2", []>;
> @@ -473,6 +475,8 @@ defm S_LOAD_DWORDX8 : SMRD_Helper <0x00000003, "S_LOAD_DWORDX8", SReg_256, v8i32
>  //def S_BUFFER_LOAD_DWORDX8 : SMRD_DWORDX8 <0x0000000b, "S_BUFFER_LOAD_DWORDX8", []>;
>  //def S_BUFFER_LOAD_DWORDX16 : SMRD_DWORDX16 <0x0000000c, "S_BUFFER_LOAD_DWORDX16", []>;
>  
> +} // mayLoad = 1
> +
>  //def S_MEMTIME : SMRD_ <0x0000001e, "S_MEMTIME", []>;
>  //def S_DCACHE_INV : SMRD_ <0x0000001f, "S_DCACHE_INV", []>;
>  //def IMAGE_LOAD : MIMG_NoPattern_ <"IMAGE_LOAD", 0x00000000>;
> @@ -1361,4 +1365,33 @@ def : Pat <(f32 (IL_mad AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2)),
>             (V_MAD_LEGACY_F32 AllReg_32:$src0, VReg_32:$src1, VReg_32:$src2,
>              0, 0, 0, 0)>;
>  
> +/********** ================== **********/
> +/**********   SMRD Patterns    **********/
> +/********** ================== **********/
> +
> +multiclass SMRD_Pattern <SMRD Instr_IMM, SMRD Instr_SGPR, ValueType vt> {
> +  // 1. Offset as 8bit DWORD immediate
> +  def : Pat <
> +    (constant_load (SIadd64bit32bit SReg_64:$sbase, IMM8bitDWORD:$offset)),
> +    (vt (Instr_IMM SReg_64:$sbase, IMM8bitDWORD:$offset))
> +  >;
> +
> +  // 2. Offset loaded in an 32bit SGPR
> +  def : Pat <
> +    (constant_load (SIadd64bit32bit SReg_64:$sbase, imm:$offset)),
> +    (vt (Instr_SGPR SReg_64:$sbase, (S_MOV_IMM_I32 imm:$offset)))
> +  >;
> +
> +  // 3. No offset at all
> +  def : Pat <
> +    (constant_load SReg_64:$sbase),
> +    (vt (Instr_IMM SReg_64:$sbase, 0))
> +  >;
> +}
> +
> +defm : SMRD_Pattern <S_LOAD_DWORD_IMM, S_LOAD_DWORD_SGPR, f32>;
> +defm : SMRD_Pattern <S_LOAD_DWORD_IMM, S_LOAD_DWORD_SGPR, i32>;
> +defm : SMRD_Pattern <S_LOAD_DWORDX4_IMM, S_LOAD_DWORDX4_SGPR, v4i32>;
> +defm : SMRD_Pattern <S_LOAD_DWORDX8_IMM, S_LOAD_DWORDX8_SGPR, v8i32>;
> +
>  } // End isSI predicate
> -- 
> 1.7.9.5
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev


More information about the mesa-dev mailing list