[Mesa-dev] [PATCH] radeon/llvm: do not use magic number for resourceId

Tom Stellard tom at stellard.net
Tue Nov 27 06:25:08 PST 2012


On Fri, Nov 23, 2012 at 10:32:33PM +0100, Vincent Lejeune wrote:
> ---
>  lib/Target/AMDGPU/AMDGPUIntrinsics.td              | 16 ++++-----
>  .../AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp      |  7 ++--
>  lib/Target/AMDGPU/R600ISelLowering.cpp             | 14 +++++---
>  lib/Target/AMDGPU/R600Instructions.td              | 40 +++++++++++-----------
>  4 files changed, 42 insertions(+), 35 deletions(-)
> 

This is a nice clean up, just one coding style issue.  With that fixed:
Reviewed-by: Tom Stellard <thomas.stellard at amd.com>

> diff --git a/lib/Target/AMDGPU/AMDGPUIntrinsics.td b/lib/Target/AMDGPU/AMDGPUIntrinsics.td
> index 1f2428a..2ba2d4b 100644
> --- a/lib/Target/AMDGPU/AMDGPUIntrinsics.td
> +++ b/lib/Target/AMDGPU/AMDGPUIntrinsics.td
> @@ -36,15 +36,15 @@ let TargetPrefix = "AMDGPU", isTarget = 1 in {
>    def int_AMDGPU_sle : Intrinsic<[llvm_float_ty], [llvm_float_ty, llvm_float_ty], [IntrNoMem]>;
>    def int_AMDGPU_sne : Intrinsic<[llvm_float_ty], [llvm_float_ty, llvm_float_ty], [IntrNoMem]>;
>    def int_AMDGPU_mullit : Intrinsic<[llvm_v4f32_ty], [llvm_float_ty, llvm_float_ty, llvm_float_ty], [IntrNoMem]>;
> -  def int_AMDGPU_tex : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_txb : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_txf : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_txq : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_txd : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_v4f32_ty, llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_txl : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_tex : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_txb : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_txf : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_txq : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_txd : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_v4f32_ty, llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_txl : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
>    def int_AMDGPU_trunc : Intrinsic<[llvm_float_ty], [llvm_float_ty], [IntrNoMem]>;
> -  def int_AMDGPU_ddx : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> -  def int_AMDGPU_ddy : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_ddx : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> +  def int_AMDGPU_ddy : Intrinsic<[llvm_v4f32_ty], [llvm_v4f32_ty, llvm_i32_ty, llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
>    def int_AMDGPU_imax : Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
>    def int_AMDGPU_imin : Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
>    def int_AMDGPU_umax : Intrinsic<[llvm_i32_ty], [llvm_i32_ty, llvm_i32_ty], [IntrNoMem]>;
> diff --git a/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp b/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> index 5178157..b9e9b1a 100644
> --- a/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> +++ b/lib/Target/AMDGPU/MCTargetDesc/R600MCCodeEmitter.cpp
> @@ -339,8 +339,9 @@ void R600MCCodeEmitter::EmitTexInstr(const MCInst &MI,
>    unsigned opcode = MI.getOpcode();
>    bool hasOffsets = (opcode == AMDGPU::TEX_LD);
>    unsigned op_offset = hasOffsets ? 3 : 0;
> -  int64_t sampler = MI.getOperand(op_offset+2).getImm();
> -  int64_t textureType = MI.getOperand(op_offset+3).getImm();
> +  int64_t resource = MI.getOperand(op_offset+2).getImm();
> +  int64_t sampler = MI.getOperand(op_offset+3).getImm();
> +  int64_t textureType = MI.getOperand(op_offset+4).getImm();
>    unsigned srcSelect[4] = {0, 1, 2, 3};
>

While we're changing these, we should fix the variable names to
conform with LLVM coding standards.
  
>    // Emit instruction type
> @@ -350,7 +351,7 @@ void R600MCCodeEmitter::EmitTexInstr(const MCInst &MI,
>    EmitByte(getBinaryCodeForInstr(MI, Fixups), OS);
>  
>    // XXX: Emit resource id r600_shader.c uses sampler + 1.  Why?
> -  EmitByte(sampler + 1 + 1, OS);
> +  EmitByte(resource, OS);
>  
>    // Emit source register
>    EmitByte(getHWReg(MI.getOperand(1).getReg()), OS);
> diff --git a/lib/Target/AMDGPU/R600ISelLowering.cpp b/lib/Target/AMDGPU/R600ISelLowering.cpp
> index 3adc5d6..a9881e0 100644
> --- a/lib/Target/AMDGPU/R600ISelLowering.cpp
> +++ b/lib/Target/AMDGPU/R600ISelLowering.cpp
> @@ -234,16 +234,19 @@ MachineBasicBlock * R600TargetLowering::EmitInstrWithCustomInserter(
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SET_GRADIENTS_H), t0)
>                .addOperand(MI->getOperand(3))
>                .addOperand(MI->getOperand(4))
> -              .addOperand(MI->getOperand(5));
> +              .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6));
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SET_GRADIENTS_V), t1)
>                .addOperand(MI->getOperand(2))
>                .addOperand(MI->getOperand(4))
> -              .addOperand(MI->getOperand(5));
> +              .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6));
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SAMPLE_G))
>                .addOperand(MI->getOperand(0))
>                .addOperand(MI->getOperand(1))
>                .addOperand(MI->getOperand(4))
>                .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6))
>                .addReg(t0, RegState::Implicit)
>                .addReg(t1, RegState::Implicit);
>        break;
> @@ -256,16 +259,19 @@ MachineBasicBlock * R600TargetLowering::EmitInstrWithCustomInserter(
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SET_GRADIENTS_H), t0)
>                .addOperand(MI->getOperand(3))
>                .addOperand(MI->getOperand(4))
> -              .addOperand(MI->getOperand(5));
> +              .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6));
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SET_GRADIENTS_V), t1)
>                .addOperand(MI->getOperand(2))
>                .addOperand(MI->getOperand(4))
> -              .addOperand(MI->getOperand(5));
> +              .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6));
>        BuildMI(*BB, I, BB->findDebugLoc(I), TII->get(AMDGPU::TEX_SAMPLE_C_G))
>                .addOperand(MI->getOperand(0))
>                .addOperand(MI->getOperand(1))
>                .addOperand(MI->getOperand(4))
>                .addOperand(MI->getOperand(5))
> +              .addOperand(MI->getOperand(6))
>                .addReg(t0, RegState::Implicit)
>                .addReg(t1, RegState::Implicit);
>        break;
> diff --git a/lib/Target/AMDGPU/R600Instructions.td b/lib/Target/AMDGPU/R600Instructions.td
> index a8be1a3..3841bff 100644
> --- a/lib/Target/AMDGPU/R600Instructions.td
> +++ b/lib/Target/AMDGPU/R600Instructions.td
> @@ -331,8 +331,8 @@ class R600_TEX <bits<11> inst, string opName, list<dag> pattern,
>                  InstrItinClass itin = AnyALU> :
>    InstR600 <inst,
>            (outs R600_Reg128:$dst),
> -          (ins R600_Reg128:$src0, i32imm:$src1, i32imm:$src2),
> -          !strconcat(opName, "$dst, $src0, $src1, $src2"),
> +          (ins R600_Reg128:$src0, i32imm:$resourceId, i32imm:$samplerId, i32imm:$textureTarget),
> +          !strconcat(opName, "$dst, $src0, $resourceId, $samplerId, $textureTarget"),
>            pattern,
>            itin>{
>      let Inst {10-0} = inst;
> @@ -713,25 +713,25 @@ def CNDGT_INT : R600_3OP <
>  
>  def TEX_LD : R600_TEX <
>    0x03, "TEX_LD",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txf R600_Reg128:$src0, imm:$src1, imm:$src2, imm:$src3, imm:$src4, imm:$src5))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txf R600_Reg128:$src0, imm:$src1, imm:$src2, imm:$src3, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  > {
> -let AsmString = "TEX_LD $dst, $src0, $src1, $src2, $src3, $src4, $src5";
> -let InOperandList = (ins R600_Reg128:$src0, i32imm:$src1, i32imm:$src2, i32imm:$src3, i32imm:$src4, i32imm:$src5);
> +let AsmString = "TEX_LD $dst, $src0, $src1, $src2, $src3, $resourceId, $samplerId, $textureTarget";
> +let InOperandList = (ins R600_Reg128:$src0, i32imm:$src1, i32imm:$src2, i32imm:$src3, i32imm:$resourceId, i32imm:$samplerId, i32imm:$textureTarget);
>  }
>  
>  def TEX_GET_TEXTURE_RESINFO : R600_TEX <
>    0x04, "TEX_GET_TEXTURE_RESINFO",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txq R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txq R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_GET_GRADIENTS_H : R600_TEX <
>    0x07, "TEX_GET_GRADIENTS_H",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_ddx R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_ddx R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_GET_GRADIENTS_V : R600_TEX <
>    0x08, "TEX_GET_GRADIENTS_V",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_ddy R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_ddy R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_SET_GRADIENTS_H : R600_TEX <
> @@ -746,32 +746,32 @@ def TEX_SET_GRADIENTS_V : R600_TEX <
>  
>  def TEX_SAMPLE : R600_TEX <
>    0x10, "TEX_SAMPLE",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_tex R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_tex R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_C : R600_TEX <
>    0x18, "TEX_SAMPLE_C",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_tex R600_Reg128:$src0, imm:$src1, TEX_SHADOW:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_tex R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, TEX_SHADOW:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_L : R600_TEX <
>    0x11, "TEX_SAMPLE_L",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txl R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txl R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_C_L : R600_TEX <
>    0x19, "TEX_SAMPLE_C_L",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txl R600_Reg128:$src0, imm:$src1, TEX_SHADOW:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txl R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, TEX_SHADOW:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_LB : R600_TEX <
>    0x12, "TEX_SAMPLE_LB",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txb R600_Reg128:$src0, imm:$src1, imm:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txb R600_Reg128:$src0,imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_C_LB : R600_TEX <
>    0x1A, "TEX_SAMPLE_C_LB",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txb R600_Reg128:$src0, imm:$src1, TEX_SHADOW:$src2))]
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txb R600_Reg128:$src0, imm:$resourceId, imm:$samplerId, TEX_SHADOW:$textureTarget))]
>  >;
>  
>  def TEX_SAMPLE_G : R600_TEX <
> @@ -1508,16 +1508,16 @@ def RESERVE_REG : AMDGPUShaderInst <
>  
>  def TXD: AMDGPUShaderInst <
>    (outs R600_Reg128:$dst),
> -  (ins R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, i32imm:$src3, i32imm:$src4),
> -  "TXD $dst, $src0, $src1, $src2, $src3, $src4",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txd R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, imm:$src3, imm:$src4))]
> +  (ins R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, i32imm:$resourceId, i32imm:$samplerId, i32imm:$textureTarget),
> +  "TXD $dst, $src0, $src1, $src2, $resourceId, $samplerId, $textureTarget",
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txd R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, imm:$resourceId, imm:$samplerId, imm:$textureTarget))]
>  >;
>  
>  def TXD_SHADOW: AMDGPUShaderInst <
>    (outs R600_Reg128:$dst),
> -  (ins R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, i32imm:$src3, i32imm:$src4),
> -  "TXD_SHADOW $dst, $src0, $src1, $src2, $src3, $src4",
> -  [(set R600_Reg128:$dst, (int_AMDGPU_txd R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, imm:$src3, TEX_SHADOW:$src4))]
> +  (ins R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, i32imm:$resourceId, i32imm:$samplerId, i32imm:$textureTarget),
> +  "TXD_SHADOW $dst, $src0, $src1, $src2, $resourceId, $samplerId, $textureTarget",
> +  [(set R600_Reg128:$dst, (int_AMDGPU_txd R600_Reg128:$src0, R600_Reg128:$src1, R600_Reg128:$src2, imm:$resourceId, imm:$samplerId, TEX_SHADOW:$textureTarget))]
>  >;
>  
>  } // End isPseudo = 1
> -- 
> 1.7.11.7
> 
> _______________________________________________
> 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