[Mesa-dev] [PATCH] gm107/ir: Add stg, ldg instructions and function for checking offset length

[Mark Menzynski]

On Fri, Jul 19, 2019 at 5:04 PM Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>
> On Fri, Jul 19, 2019 at 10:57 AM Mark Menzynski <mmenzyns at redhat.com> wrote:
> >
> > Nvidia actively uses these instructions, maybe they are better in
> > something.
> > Long offset checking function was made because these functions only have 24 bit
> > address offsets.
> >
> > Signed-off-by: Mark Menzynski <mmenzyns at redhat.com>
> > ---
> >  .../nouveau/codegen/nv50_ir_emit_gm107.cpp    | 36 +++++++++++++++++++
> >  1 file changed, 36 insertions(+)
> >
> > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> > index 6eefe8f0025..c01a3017ba9 100644
> > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
> > @@ -87,6 +87,7 @@ private:
> >     inline void emitADDR(int, int, int, int, const ValueRef &);
> >     inline void emitCBUF(int, int, int, int, int, const ValueRef &);
> >     inline bool longIMMD(const ValueRef &);
> > +   inline bool longOffset(const ValueRef &);
> >     inline void emitIMMD(int, int, const ValueRef &);
> >
> >     void emitCond3(int, CondCode);
> > @@ -174,9 +175,11 @@ private:
> >     void emitLDC();
> >     void emitLDL();
> >     void emitLDS();
> > +   void emitLDG();
> >     void emitLD();
> >     void emitSTL();
> >     void emitSTS();
> > +   void emitSTG();
> >     void emitST();
> >     void emitALD();
> >     void emitAST();
> > @@ -333,6 +336,17 @@ CodeEmitterGM107::longIMMD(const ValueRef &ref)
> >     return false;
> >  }
> >
> > +bool
> > +CodeEmitterGM107::longOffset(const ValueRef &ref)
> > +{
> > +   // TODO: check for other files as well?
> > +   if (ref.getFile() != FILE_MEMORY_GLOBAL)
> > +      return false;
>
> I haven't seen the uses (best to send stuff like this as a series),
> but you're saying that if it's not global memory, then it's not a long
> offset? I suspect in the caller it should be more like
>
> assert(file == global || !long offset) or something.
>
This is how I used it for Load. Store was used the same way. I have
not sent it because we didn't found any noticeable changes with that:

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
index c01a3017ba9..f632178138b 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
@@ -3603,7 +3603,12 @@ CodeEmitterGM107::emitInstruction(Instruction *i)
       case FILE_MEMORY_CONST : emitLDC(); break;
       case FILE_MEMORY_LOCAL : emitLDL(); break;
       case FILE_MEMORY_SHARED: emitLDS(); break;
-      case FILE_MEMORY_GLOBAL: emitLD(); break;
+      case FILE_MEMORY_GLOBAL:
+         if (longOffset(insn->src(0)))
+            emitLD();
+         else
+            emitLDG();
+         break;
       default:
          assert(!"invalid load");
          emitNOP();
> > +
> > +   int32_t offset = ref.get()->reg.data.offset;
> > +   return offset >  0x7fffff || offset < -0x800000;
>
> You have two spaces after the >. Remove one of them.
>
> > +}
> > +
> >  void
> >  CodeEmitterGM107::emitIMMD(int pos, int len, const ValueRef &ref)
> >  {
> > @@ -2414,6 +2428,17 @@ CodeEmitterGM107::emitLDS()
> >     emitGPR  (0x00, insn->def(0));
> >  }
> >
> > +void
> > +CodeEmitterGM107::emitLDG()
> > +{
> > +   emitInsn (0xeed00000);
> > +   emitLDSTs(0x30, insn->dType);
> > +   emitLDSTc(0x2e);
> > +   emitField(0x2d, 1, insn->src(0).getIndirect(0)->getSize() == 8);
>
> I didn't look, but we don't do something a bit more subtle on the
> other ones, like checking if there's an indirect access in the first
> place? With g[], it almost exclusively will be, but still...
>
> > +   emitADDR (0x08, 0x14, 24, 0, insn->src(0));
> > +   emitGPR  (0x00, insn->def(0));
> > +}
> > +
> >  void
> >  CodeEmitterGM107::emitLD()
> >  {
> > @@ -2445,6 +2470,17 @@ CodeEmitterGM107::emitSTS()
> >     emitGPR  (0x00, insn->src(1));
> >  }
> >
> > +void
> > +CodeEmitterGM107::emitSTG()
> > +{
> > +   emitInsn (0xeed80000);
> > +   emitLDSTs(0x30, insn->dType);
> > +   emitLDSTc(0x2e);
> > +   emitField(0x2d, 1, insn->src(0).getIndirect(0)->getSize() == 8);
> > +   emitADDR (0x08, 0x14, 24, 0, insn->src(0));
> > +   emitGPR  (0x00, insn->src(1));
> > +}
> > +
> >  void
> >  CodeEmitterGM107::emitST()
> >  {
> > --
> > 2.21.0
> >
> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev