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

[Ilia Mirkin]

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.

> +
> +   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