[Mesa-dev] [PATCH v3] R600/SI: Fix int_SI_fs_interp_constant

Christian König deathsimple at vodafone.de
Thu Feb 14 10:38:04 PST 2013


Am 14.02.2013 19:30, schrieb Michel Dänzer:
> From: Michel Dänzer <michel.daenzer at amd.com>
>
> The important fix is that the constant interpolation value is stored in the
> parameter slot P0, which is encoded as 2.
>
> In addition, drop the SI_INTERP_CONST pseudo instruction, pass the parameter
> slot as an operand to V_INTERP_MOV_F32 instead of hardcoding it there, and
> add a special operand class for the parameter slots for type checking and
> pretty printing.
>
> NOTE: This is a candidate for the Mesa stable branch.
>
> Signed-off-by: Michel Dänzer <michel.daenzer at amd.com>
Reviewed-by: Christian König <christian.koenig at amd.com>

> ---
>
> v3: Use operand class instead of register class, drop SI_INTERP_CONST
>      altogether.
>
>   lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp | 15 ++++++++++++
>   lib/Target/R600/InstPrinter/AMDGPUInstPrinter.h   |  1 +
>   lib/Target/R600/SIISelLowering.cpp                | 24 -------------------
>   lib/Target/R600/SIISelLowering.h                  |  2 --
>   lib/Target/R600/SIInstructions.td                 | 29 ++++++++++++++---------
>   5 files changed, 34 insertions(+), 37 deletions(-)
>
> diff --git a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp
> index fb17ab7..d6450a0 100644
> --- a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp
> +++ b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.cpp
> @@ -40,6 +40,21 @@ void AMDGPUInstPrinter::printOperand(const MCInst *MI, unsigned OpNo,
>     }
>   }
>   
> +void AMDGPUInstPrinter::printInterpSlot(const MCInst *MI, unsigned OpNum,
> +                                        raw_ostream &O) {
> +  unsigned Imm = MI->getOperand(OpNum).getImm();
> +
> +  if (Imm == 2) {
> +    O << "P0";
> +  } else if (Imm == 1) {
> +    O << "P20";
> +  } else if (Imm == 0) {
> +    O << "P10";
> +  } else {
> +    assert(!"Invalid interpolation parameter slot");
> +  }
> +}
> +
>   void AMDGPUInstPrinter::printMemOperand(const MCInst *MI, unsigned OpNo,
>                                           raw_ostream &O) {
>     printOperand(MI, OpNo, O);
> diff --git a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.h b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.h
> index e775c4c..767a708 100644
> --- a/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.h
> +++ b/lib/Target/R600/InstPrinter/AMDGPUInstPrinter.h
> @@ -33,6 +33,7 @@ public:
>   
>   private:
>     void printOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
> +  void printInterpSlot(const MCInst *MI, unsigned OpNum, raw_ostream &O);
>     void printMemOperand(const MCInst *MI, unsigned OpNo, raw_ostream &O);
>     void printIfSet(const MCInst *MI, unsigned OpNo, raw_ostream &O, StringRef Asm);
>     void printAbs(const MCInst *MI, unsigned OpNo, raw_ostream &O);
> diff --git a/lib/Target/R600/SIISelLowering.cpp b/lib/Target/R600/SIISelLowering.cpp
> index 87cf596..3865d4f 100644
> --- a/lib/Target/R600/SIISelLowering.cpp
> +++ b/lib/Target/R600/SIISelLowering.cpp
> @@ -120,9 +120,6 @@ MachineBasicBlock * SITargetLowering::EmitInstrWithCustomInserter(
>     case AMDGPU::SI_INTERP:
>       LowerSI_INTERP(MI, *BB, I, MRI);
>       break;
> -  case AMDGPU::SI_INTERP_CONST:
> -    LowerSI_INTERP_CONST(MI, *BB, I, MRI);
> -    break;
>     case AMDGPU::SI_WQM:
>       LowerSI_WQM(MI, *BB, I, MRI);
>       break;
> @@ -172,27 +169,6 @@ void SITargetLowering::LowerSI_INTERP(MachineInstr *MI, MachineBasicBlock &BB,
>     MI->eraseFromParent();
>   }
>   
> -void SITargetLowering::LowerSI_INTERP_CONST(MachineInstr *MI,
> -    MachineBasicBlock &BB, MachineBasicBlock::iterator I,
> -    MachineRegisterInfo &MRI) const {
> -  MachineOperand dst = MI->getOperand(0);
> -  MachineOperand attr_chan = MI->getOperand(1);
> -  MachineOperand attr = MI->getOperand(2);
> -  MachineOperand params = MI->getOperand(3);
> -  unsigned M0 = MRI.createVirtualRegister(&AMDGPU::M0RegRegClass);
> -
> -  BuildMI(BB, I, BB.findDebugLoc(I), TII->get(AMDGPU::S_MOV_B32), M0)
> -          .addOperand(params);
> -
> -  BuildMI(BB, I, BB.findDebugLoc(I), TII->get(AMDGPU::V_INTERP_MOV_F32))
> -          .addOperand(dst)
> -          .addOperand(attr_chan)
> -          .addOperand(attr)
> -          .addReg(M0);
> -
> -  MI->eraseFromParent();
> -}
> -
>   void SITargetLowering::LowerSI_V_CNDLT(MachineInstr *MI, MachineBasicBlock &BB,
>       MachineBasicBlock::iterator I, MachineRegisterInfo & MRI) const {
>     unsigned VCC = MRI.createVirtualRegister(&AMDGPU::SReg_64RegClass);
> diff --git a/lib/Target/R600/SIISelLowering.h b/lib/Target/R600/SIISelLowering.h
> index 8528c24..f4bc94d 100644
> --- a/lib/Target/R600/SIISelLowering.h
> +++ b/lib/Target/R600/SIISelLowering.h
> @@ -27,8 +27,6 @@ class SITargetLowering : public AMDGPUTargetLowering {
>                 MachineBasicBlock::iterator I, unsigned Opocde) const;
>     void LowerSI_INTERP(MachineInstr *MI, MachineBasicBlock &BB,
>                 MachineBasicBlock::iterator I, MachineRegisterInfo & MRI) const;
> -  void LowerSI_INTERP_CONST(MachineInstr *MI, MachineBasicBlock &BB,
> -              MachineBasicBlock::iterator I, MachineRegisterInfo &MRI) const;
>     void LowerSI_WQM(MachineInstr *MI, MachineBasicBlock &BB,
>                 MachineBasicBlock::iterator I, MachineRegisterInfo & MRI) const;
>     void LowerSI_V_CNDLT(MachineInstr *MI, MachineBasicBlock &BB,
> diff --git a/lib/Target/R600/SIInstructions.td b/lib/Target/R600/SIInstructions.td
> index 10d5bae..fa6367c 100644
> --- a/lib/Target/R600/SIInstructions.td
> +++ b/lib/Target/R600/SIInstructions.td
> @@ -11,6 +11,17 @@
>   // that are not yet supported remain commented out.
>   //===----------------------------------------------------------------------===//
>   
> +class InterpSlots {
> +int P0 = 2;
> +int P10 = 0;
> +int P20 = 1;
> +}
> +def INTERP : InterpSlots;
> +
> +def InterpSlot : Operand<i32> {
> +  let PrintMethod = "printInterpSlot";
> +}
> +
>   def isSI : Predicate<"Subtarget.device()"
>                               "->getGeneration() == AMDGPUDeviceInfo::HD7XXX">;
>   
> @@ -683,10 +694,9 @@ def V_INTERP_P2_F32 : VINTRP <
>   def V_INTERP_MOV_F32 : VINTRP <
>     0x00000002,
>     (outs VReg_32:$dst),
> -  (ins i32imm:$attr_chan, i32imm:$attr, M0Reg:$m0),
> -  "V_INTERP_MOV_F32",
> +  (ins InterpSlot:$src0, i32imm:$attr_chan, i32imm:$attr, M0Reg:$m0),
> +  "V_INTERP_MOV_F32 $dst, $src0, $attr_chan, $attr",
>     []> {
> -  let VSRC = 0;
>     let DisableEncoding = "$m0";
>   }
>   
> @@ -1081,14 +1091,6 @@ def SI_INTERP : InstSI <
>     []
>   >;
>   
> -def SI_INTERP_CONST : InstSI <
> -  (outs VReg_32:$dst),
> -  (ins i32imm:$attr_chan, i32imm:$attr, SReg_32:$params),
> -  "SI_INTERP_CONST $dst, $attr_chan, $attr, $params",
> -  [(set VReg_32:$dst, (int_SI_fs_interp_constant imm:$attr_chan,
> -                                                 imm:$attr, SReg_32:$params))]
> ->;
> -
>   def SI_WQM : InstSI <
>     (outs),
>     (ins),
> @@ -1324,6 +1326,11 @@ def : Pat <
>   /********** ===================== **********/
>   
>   def : Pat <
> +  (int_SI_fs_interp_constant imm:$attr_chan, imm:$attr, SReg_32:$params),
> +  (V_INTERP_MOV_F32 INTERP.P0, imm:$attr_chan, imm:$attr, SReg_32:$params)
> +>;
> +
> +def : Pat <
>     (int_SI_fs_interp_linear_center imm:$attr_chan, imm:$attr, SReg_32:$params),
>     (SI_INTERP (f32 LINEAR_CENTER_I), (f32 LINEAR_CENTER_J), imm:$attr_chan,
>                imm:$attr, SReg_32:$params)



More information about the mesa-dev mailing list