gm107/ir: Add stg, ldg instructions and function for checking offset length

Submitted by Mark Menzynski on July 19, 2019, 2:57 p.m.

Details

Message ID 20190719145713.28197-1-mmenzyns@redhat.com
State New
Headers show
Series "gm107/ir: Add stg, ldg instructions and function for checking offset length" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Mark Menzynski July 19, 2019, 2:57 p.m.
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@redhat.com>
---
 .../nouveau/codegen/nv50_ir_emit_gm107.cpp    | 36 +++++++++++++++++++
 1 file changed, 36 insertions(+)

Patch hide | download patch | download mbox

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;
+
+   int32_t offset = ref.get()->reg.data.offset;
+   return offset >  0x7fffff || offset < -0x800000;
+}
+
 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);
+   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()
 {

Comments

On Fri, Jul 19, 2019 at 10:57 AM Mark Menzynski <mmenzyns@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@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.

> +
> +   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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> > @@ -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...

It's done same in the original store and load functions.

> > _______________________________________________
> > mesa-dev mailing list
> > mesa-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev