[PATCH] AMDGPU/SI: Don't set DATA_FORMAT if ADD_TID_ENABLE is set

[Tom Stellard via llvm-commits]

On Tue, Sep 29, 2015 at 05:32:59PM +0200, Christian König wrote:
> On 29.09.2015 17:29, Tom Stellard via llvm-commits wrote:
> >On Tue, Sep 29, 2015 at 04:56:53PM +0200, Marek Olšák via llvm-commits wrote:
> >>From: Marek Olšák <marek.olsak at amd.com>
> >>
> >>to prevent setting a huge stride, because DATA_FORMAT has a different
> >>meaning if ADD_TID_ENABLE is set.
> >>
> >>This is a candidate for stable llvm 3.7.
> >>---
> >>  lib/Target/AMDGPU/SIISelLowering.cpp       |  4 +---
> >>  lib/Target/AMDGPU/SIInstrInfo.cpp          | 14 ++++++++++++++
> >>  lib/Target/AMDGPU/SIInstrInfo.h            |  2 +-
> >>  lib/Target/AMDGPU/SIPrepareScratchRegs.cpp |  7 +++----
> >>  4 files changed, 19 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/lib/Target/AMDGPU/SIISelLowering.cpp b/lib/Target/AMDGPU/SIISelLowering.cpp
> >>index 0bae789..b4b439c 100644
> >>--- a/lib/Target/AMDGPU/SIISelLowering.cpp
> >>+++ b/lib/Target/AMDGPU/SIISelLowering.cpp
> >>@@ -2257,10 +2257,8 @@ MachineSDNode *SITargetLowering::buildScratchRSRC(SelectionDAG &DAG,
> >>                                                    SDValue Ptr) const {
> >>    const SIInstrInfo *TII =
> >>        static_cast<const SIInstrInfo *>(Subtarget->getInstrInfo());
> >>-  uint64_t Rsrc = TII->getDefaultRsrcDataFormat() | AMDGPU::RSRC_TID_ENABLE |
> >>-                  0xffffffff; // Size
> >>-  return buildRSRC(DAG, DL, Ptr, 0, Rsrc);
> >>+  return buildRSRC(DAG, DL, Ptr, 0, TII->getScratchRsrcWords23());
> >>  }
> >>  SDValue SITargetLowering::CreateLiveInRegister(SelectionDAG &DAG,
> >>diff --git a/lib/Target/AMDGPU/SIInstrInfo.cpp b/lib/Target/AMDGPU/SIInstrInfo.cpp
> >>index b667dc2..7b10f32 100644
> >>--- a/lib/Target/AMDGPU/SIInstrInfo.cpp
> >>+++ b/lib/Target/AMDGPU/SIInstrInfo.cpp
> >>@@ -2781,3 +2781,17 @@ uint64_t SIInstrInfo::getDefaultRsrcDataFormat() const {
> >>    return RsrcDataFormat;
> >>  }
> >>+
> >>+uint64_t SIInstrInfo::getScratchRsrcWords23() const {
> >>+  uint64_t Rsrc23 = getDefaultRsrcDataFormat() |
> >>+                    AMDGPU::RSRC_TID_ENABLE |
> >>+                    0xffffffff; // Size;
> >>+
> >>+  /* If TID_ENABLE is set, DATA_FORMAT specifies stride bits [14:17].
> >>+   * Clear them unless we want a huge stride.
> >>+   */
> >Need to use C++ comments: //
> >
> >
> >>+  if (ST.getGeneration() >= AMDGPUSubtarget::VOLCANIC_ISLANDS)
> >>+    Rsrc23 &= ~AMDGPU::RSRC_DATA_FORMAT;
> >>+
> >Is there a reason why we need to do this only for VI?
> 
> Yeah, that new encoding only applies for VI. SI/CIK don't have that
> feature of an extended stride.
> 

OK, LGTM.

> Regards,
> Christian.
> 
> >
> >-Tom
> >
> >>+  return Rsrc23;
> >>+}
> >>diff --git a/lib/Target/AMDGPU/SIInstrInfo.h b/lib/Target/AMDGPU/SIInstrInfo.h
> >>index f4b2f2d..ba8b89a 100644
> >>--- a/lib/Target/AMDGPU/SIInstrInfo.h
> >>+++ b/lib/Target/AMDGPU/SIInstrInfo.h
> >>@@ -356,7 +356,7 @@ public:
> >>    }
> >>    uint64_t getDefaultRsrcDataFormat() const;
> >>-
> >>+  uint64_t getScratchRsrcWords23() const;
> >>  };
> >>  namespace AMDGPU {
> >>diff --git a/lib/Target/AMDGPU/SIPrepareScratchRegs.cpp b/lib/Target/AMDGPU/SIPrepareScratchRegs.cpp
> >>index f6e57a6..f4a5ac1 100644
> >>--- a/lib/Target/AMDGPU/SIPrepareScratchRegs.cpp
> >>+++ b/lib/Target/AMDGPU/SIPrepareScratchRegs.cpp
> >>@@ -138,8 +138,7 @@ bool SIPrepareScratchRegs::runOnMachineFunction(MachineFunction &MF) {
> >>        unsigned ScratchRsrcReg =
> >>            RS.scavengeRegister(&AMDGPU::SReg_128RegClass, 0);
> >>-      uint64_t Rsrc = AMDGPU::RSRC_DATA_FORMAT | AMDGPU::RSRC_TID_ENABLE |
> >>-                      0xffffffff; // Size
> >>+      uint64_t Rsrc23 = TII->getScratchRsrcWords23();
> >>        unsigned Rsrc0 = TRI->getSubReg(ScratchRsrcReg, AMDGPU::sub0);
> >>        unsigned Rsrc1 = TRI->getSubReg(ScratchRsrcReg, AMDGPU::sub1);
> >>@@ -155,11 +154,11 @@ bool SIPrepareScratchRegs::runOnMachineFunction(MachineFunction &MF) {
> >>                .addReg(ScratchRsrcReg, RegState::ImplicitDefine);
> >>        BuildMI(MBB, I, DL, TII->get(AMDGPU::S_MOV_B32), Rsrc2)
> >>-              .addImm(Rsrc & 0xffffffff)
> >>+              .addImm(Rsrc23 & 0xffffffff)
> >>                .addReg(ScratchRsrcReg, RegState::ImplicitDefine);
> >>        BuildMI(MBB, I, DL, TII->get(AMDGPU::S_MOV_B32), Rsrc3)
> >>-              .addImm(Rsrc >> 32)
> >>+              .addImm(Rsrc23 >> 32)
> >>                .addReg(ScratchRsrcReg, RegState::ImplicitDefine);
> >>        // Scratch Offset
> >>-- 
> >>2.1.4
> >>
> >>_______________________________________________
> >>llvm-commits mailing list
> >>llvm-commits at lists.llvm.org
> >>http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >_______________________________________________
> >llvm-commits mailing list
> >llvm-commits at lists.llvm.org
> >http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>