[Mesa-dev] [PATCH v2] R600/SI: Fix INTERP_CONST.

Michel Dänzer michel at daenzer.net
Thu Feb 14 09:07:16 PST 2013


On Mit, 2013-02-13 at 17:51 +0100, Christian König wrote: 
> Am 13.02.2013 17:07, 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, pass the parameter slot as an operand to V_INTERP_MOV_F32
> > instead of hardcoding it there, and add a special register class for the
> > parameter slots for type checking and pretty dumping.

[...]

> > diff --git a/lib/Target/R600/SIRegisterInfo.td b/lib/Target/R600/SIRegisterInfo.td
> > index ab36b87..46c8f91 100644
> > --- a/lib/Target/R600/SIRegisterInfo.td
> > +++ b/lib/Target/R600/SIRegisterInfo.td
> > @@ -23,6 +23,9 @@ def EXEC_HI : SIReg <"EXEC HI", 127>;
> >   def EXEC : SI_64<"EXEC", [EXEC_LO, EXEC_HI], 126>;
> >   def SCC : SIReg<"SCC", 253>;
> >   def M0 : SIReg <"M0", 124>;
> > +def P10 : SIReg <"P10", 0>;
> > +def P20 : SIReg <"P20", 1>;
> > +def P0 : SIReg <"P0", 2>;
> 
> I'm not sure if representing constants as registers is such a good idea. 
> From the POV of the selection DAG a register is primary the destination 
> of an operation.
> 
> Maybe using an Operand<i32> with a proper "PrintMethod" set might be 
> better, take a look at ARMInstPrinter::printThumbSRImm on how that's done.

I've come up with the patch below, but printInterpSlot doesn't seem to
end up hooked up anywhere, and indeed P0 is just dumped as '2'. Any idea
what I'm missing?


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..aa67f22 100644
--- a/lib/Target/R600/SIInstructions.td
+++ b/lib/Target/R600/SIInstructions.td
@@ -11,6 +11,20 @@
 // 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>, PatLeaf<(imm), [{
+  uint64_t Imm = N->getZExtValue();
+  return Imm >= 0 && Imm <= 2;
+}]> {
+  let PrintMethod = "printInterpSlot";
+}
+
 def isSI : Predicate<"Subtarget.device()"
                             "->getGeneration() == AMDGPUDeviceInfo::HD7XXX">;
 
@@ -683,10 +697,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),
+  (ins InterpSlot:$src0, i32imm:$attr_chan, i32imm:$attr, M0Reg:$m0),
   "V_INTERP_MOV_F32",
   []> {
-  let VSRC = 0;
   let DisableEncoding = "$m0";
 }
 
@@ -1081,14 +1094,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 +1329,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)


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer


More information about the mesa-dev mailing list