thr3ads.net - Nouveau - [Nouveau] [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE

If this information is useful, please help other people find it:
Share via:

Hans de Goede

2016-Mar-17 09:13 UTC

[Nouveau] [PATCH mesa v2 1/3] nouveau: codegen: Disable more old resource handling code

Commit c3083c7082 ("nv50/ir: add support for BUFFER accesses")
disabled /
commented out some of the old resource handling code, but not all of it.

Effectively all of it is dead already, if we ever enter the old code
paths in handeLOAD / handleSTORE / handleATOM we will get an exception
due to trying to access the now always zero-sized resources vector.

Disable all the dead code.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
Changes in v2:
-Split out assert() on getFile() != BUFFER/MEMORY into a separate patch
-Split out removal of TGSI_RESOURCE_* defines into a separate patch
---
 src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index 1e91ad3..41eb4e3 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -856,12 +856,14 @@ public:
    };
    std::vector<TextureView> textureViews;
 
+   /*
    struct Resource {
       uint8_t target; // TGSI_TEXTURE_*
       bool raw;
       uint8_t slot; // $surface index
    };
    std::vector<Resource> resources;
+   */
 
    struct MemoryFile {
       uint8_t mem_type; // TGSI_MEMORY_TYPE_*
@@ -1419,8 +1421,8 @@ private:
    void handleLIT(Value *dst0[4]);
    void handleUserClipPlanes();
 
-   Symbol *getResourceBase(int r);
-   void getResourceCoords(std::vector<Value *>&, int r, int s);
+   // Symbol *getResourceBase(int r);
+   // void getResourceCoords(std::vector<Value *>&, int r, int s);
 
    void handleLOAD(Value *dst0[4]);
    void handleSTORE();
@@ -2161,6 +2163,7 @@ Converter::handleLIT(Value *dst0[4])
    }
 }
 
+/* Keep this around for now as reference when adding img support
 static inline bool
 isResourceSpecial(const int r)
 {
@@ -2256,6 +2259,7 @@ partitionLoadStore(uint8_t comp[2], uint8_t size[2],
uint8_t mask)
    }
    return n + 1;
 }
+*/
 
 // For raw loads, granularity is 4 byte.
 // Usage of the texture read mask on OP_SULDP is not allowed.
@@ -2290,6 +2294,7 @@ Converter::handleLOAD(Value *dst0[4])
       return;
    }
 
+/* Keep this around for now as reference when adding img support
    getResourceCoords(off, r, 1);
 
    if (isResourceRaw(code, r)) {
@@ -2355,6 +2360,7 @@ Converter::handleLOAD(Value *dst0[4])
    FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
       if (dst0[c] != def[c])
          mkMov(dst0[c], def[tgsi.getSrc(0).getSwizzle(c)]);
+*/
 }
 
 // For formatted stores, the write mask on OP_SUSTP can be used.
@@ -2391,6 +2397,7 @@ Converter::handleSTORE()
       return;
    }
 
+/* Keep this around for now as reference when adding img support
    getResourceCoords(off, r, 0);
    src = off;
    const int s = src.size();
@@ -2438,6 +2445,7 @@ Converter::handleSTORE()
       mkTex(OP_SUSTP, getResourceTarget(code, r), code->resources[r].slot,
0,
             dummy, src)->tex.mask = tgsi.getDst(0).getMask();
    }
+*/
 }
 
 // XXX: These only work on resources with the single-component u32/s32 formats.
@@ -2484,7 +2492,7 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
       return;
    }
 
-
+/* Keep this around for now as reference when adding img support
    getResourceCoords(srcv, r, 1);
 
    if (isResourceSpecial(r)) {
@@ -2512,6 +2520,7 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
    for (int c = 0; c < 4; ++c)
       if (dst0[c])
          dst0[c] = dst; // not equal to rDst so handleInstruction will do mkMov
+*/
 }
 
 void
-- 
2.7.2

Hans de Goede

2016-Mar-17 09:13 UTC

head link

[Nouveau] [PATCH mesa v2 2/3] nouveau: codegen: Do not silently fail in handeLOAD / handleSTORE / handleATOM

handeLOAD / handleSTORE / handleATOM can only handle TGSI_FILE_BUFFER
and TGSI_FILE_MEMORY. Make things fail explictly when another
register-file is used in these functions.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
Changes in v2:
-Split out of "nouveau: codegen: Disable more old resource handling
code"
---
 .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp  | 27 ++++++++++++++--------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
index 41eb4e3..baa2e30 100644
--- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
+++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
@@ -2270,8 +2270,9 @@ Converter::handleLOAD(Value *dst0[4])
    int c;
    std::vector<Value *> off, src, ldv, def;
 
-   if (tgsi.getSrc(0).getFile() == TGSI_FILE_BUFFER ||
-       tgsi.getSrc(0).getFile() == TGSI_FILE_MEMORY) {
+   switch (tgsi.getSrc(0).getFile()) {
+   case TGSI_FILE_BUFFER:
+   case TGSI_FILE_MEMORY:
       for (c = 0; c < 4; ++c) {
          if (!dst0[c])
             continue;
@@ -2291,7 +2292,9 @@ Converter::handleLOAD(Value *dst0[4])
          if (tgsi.getSrc(0).isIndirect(0))
             ld->setIndirect(0, 1, fetchSrc(tgsi.getSrc(0).getIndirect(0), 0,
0));
       }
-      return;
+      break;
+   default:
+      assert(!"Unsupported srcFile for LOAD");
    }
 
 /* Keep this around for now as reference when adding img support
@@ -2372,8 +2375,9 @@ Converter::handleSTORE()
    int c;
    std::vector<Value *> off, src, dummy;
 
-   if (tgsi.getDst(0).getFile() == TGSI_FILE_BUFFER ||
-       tgsi.getDst(0).getFile() == TGSI_FILE_MEMORY) {
+   switch (tgsi.getDst(0).getFile()) {
+   case TGSI_FILE_BUFFER:
+   case TGSI_FILE_MEMORY:
       for (c = 0; c < 4; ++c) {
          if (!(tgsi.getDst(0).getMask() & (1 << c)))
             continue;
@@ -2394,7 +2398,9 @@ Converter::handleSTORE()
          if (tgsi.getDst(0).isIndirect(0))
             st->setIndirect(0, 1, fetchSrc(tgsi.getDst(0).getIndirect(0), 0,
0));
       }
-      return;
+      break;
+   default:
+      assert(!"Unsupported dstFile for STORE");
    }
 
 /* Keep this around for now as reference when adding img support
@@ -2460,8 +2466,9 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
    std::vector<Value *> defv;
    LValue *dst = getScratch();
 
-   if (tgsi.getSrc(0).getFile() == TGSI_FILE_BUFFER ||
-       tgsi.getSrc(0).getFile() == TGSI_FILE_MEMORY) {
+   switch (tgsi.getSrc(0).getFile()) {
+   case TGSI_FILE_BUFFER:
+   case TGSI_FILE_MEMORY:
       for (int c = 0; c < 4; ++c) {
          if (!dst0[c])
             continue;
@@ -2489,7 +2496,9 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
       for (int c = 0; c < 4; ++c)
          if (dst0[c])
             dst0[c] = dst; // not equal to rDst so handleInstruction will do
mkMov
-      return;
+      break;
+   default:
+      assert(!"Unsupported srcFile for ATOM");
    }
 
 /* Keep this around for now as reference when adding img support
-- 
2.7.2

Hans de Goede

2016-Mar-17 09:13 UTC

head link

[Nouveau] [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE_ defines

These magic file-index defines where only ever used in the nouveau code
and that no longer uses them.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
Changes in v2:
-Split out of "nouveau: codegen: Disable more old resource handling
code"
---
 src/gallium/include/pipe/p_shader_tokens.h | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/src/gallium/include/pipe/p_shader_tokens.h
b/src/gallium/include/pipe/p_shader_tokens.h
index 65d8ad9..5ef6c30 100644
--- a/src/gallium/include/pipe/p_shader_tokens.h
+++ b/src/gallium/include/pipe/p_shader_tokens.h
@@ -237,15 +237,6 @@ struct tgsi_declaration_array {
    unsigned Padding : 22;
 };
 
-/*
- * Special resources that don't need to be declared.  They map to the
- * GLOBAL/LOCAL/PRIVATE/INPUT compute memory spaces.
- */
-#define TGSI_RESOURCE_GLOBAL	0x7fff
-#define TGSI_RESOURCE_LOCAL	0x7ffe
-#define TGSI_RESOURCE_PRIVATE	0x7ffd
-#define TGSI_RESOURCE_INPUT	0x7ffc
-
 #define TGSI_IMM_FLOAT32   0
 #define TGSI_IMM_UINT32    1
 #define TGSI_IMM_INT32     2
-- 
2.7.2

Samuel Pitoiset

2016-Mar-17 09:20 UTC

head link

[Nouveau] [PATCH mesa v2 1/3] nouveau: codegen: Disable more old resource handling code

Series is:

Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>

On 03/17/2016 10:13 AM, Hans de Goede wrote:> Commit c3083c7082 ("nv50/ir: add support for BUFFER accesses")
disabled /
> commented out some of the old resource handling code, but not all of it.
>
> Effectively all of it is dead already, if we ever enter the old code
> paths in handeLOAD / handleSTORE / handleATOM we will get an exception
> due to trying to access the now always zero-sized resources vector.
>
> Disable all the dead code.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> Changes in v2:
> -Split out assert() on getFile() != BUFFER/MEMORY into a separate patch
> -Split out removal of TGSI_RESOURCE_* defines into a separate patch
> ---
>   src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 15
++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> index 1e91ad3..41eb4e3 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -856,12 +856,14 @@ public:
>      };
>      std::vector<TextureView> textureViews;
>
> +   /*
>      struct Resource {
>         uint8_t target; // TGSI_TEXTURE_*
>         bool raw;
>         uint8_t slot; // $surface index
>      };
>      std::vector<Resource> resources;
> +   */
>
>      struct MemoryFile {
>         uint8_t mem_type; // TGSI_MEMORY_TYPE_*
> @@ -1419,8 +1421,8 @@ private:
>      void handleLIT(Value *dst0[4]);
>      void handleUserClipPlanes();
>
> -   Symbol *getResourceBase(int r);
> -   void getResourceCoords(std::vector<Value *>&, int r, int s);
> +   // Symbol *getResourceBase(int r);
> +   // void getResourceCoords(std::vector<Value *>&, int r, int
s);
>
>      void handleLOAD(Value *dst0[4]);
>      void handleSTORE();
> @@ -2161,6 +2163,7 @@ Converter::handleLIT(Value *dst0[4])
>      }
>   }
>
> +/* Keep this around for now as reference when adding img support
>   static inline bool
>   isResourceSpecial(const int r)
>   {
> @@ -2256,6 +2259,7 @@ partitionLoadStore(uint8_t comp[2], uint8_t size[2],
uint8_t mask)
>      }
>      return n + 1;
>   }
> +*/
>
>   // For raw loads, granularity is 4 byte.
>   // Usage of the texture read mask on OP_SULDP is not allowed.
> @@ -2290,6 +2294,7 @@ Converter::handleLOAD(Value *dst0[4])
>         return;
>      }
>
> +/* Keep this around for now as reference when adding img support
>      getResourceCoords(off, r, 1);
>
>      if (isResourceRaw(code, r)) {
> @@ -2355,6 +2360,7 @@ Converter::handleLOAD(Value *dst0[4])
>      FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
>         if (dst0[c] != def[c])
>            mkMov(dst0[c], def[tgsi.getSrc(0).getSwizzle(c)]);
> +*/
>   }
>
>   // For formatted stores, the write mask on OP_SUSTP can be used.
> @@ -2391,6 +2397,7 @@ Converter::handleSTORE()
>         return;
>      }
>
> +/* Keep this around for now as reference when adding img support
>      getResourceCoords(off, r, 0);
>      src = off;
>      const int s = src.size();
> @@ -2438,6 +2445,7 @@ Converter::handleSTORE()
>         mkTex(OP_SUSTP, getResourceTarget(code, r),
code->resources[r].slot, 0,
>               dummy, src)->tex.mask = tgsi.getDst(0).getMask();
>      }
> +*/
>   }
>
>   // XXX: These only work on resources with the single-component u32/s32
formats.
> @@ -2484,7 +2492,7 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
>         return;
>      }
>
> -
> +/* Keep this around for now as reference when adding img support
>      getResourceCoords(srcv, r, 1);
>
>      if (isResourceSpecial(r)) {
> @@ -2512,6 +2520,7 @@ Converter::handleATOM(Value *dst0[4], DataType ty,
uint16_t subOp)
>      for (int c = 0; c < 4; ++c)
>         if (dst0[c])
>            dst0[c] = dst; // not equal to rDst so handleInstruction will do
mkMov
> +*/
>   }
>
>   void
>

Marek Olšák

2016-Mar-17 15:20 UTC

head link

[Nouveau] [Mesa-dev] [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE_ defines

Reviewed-by: Marek Olšák <marek.olsak at amd.com>

Marek

On Thu, Mar 17, 2016 at 10:13 AM, Hans de Goede <hdegoede at redhat.com>
wrote:> These magic file-index defines where only ever used in the nouveau code
> and that no longer uses them.
>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
> Changes in v2:
> -Split out of "nouveau: codegen: Disable more old resource handling
code"
> ---
>  src/gallium/include/pipe/p_shader_tokens.h | 9 ---------
>  1 file changed, 9 deletions(-)
>
> diff --git a/src/gallium/include/pipe/p_shader_tokens.h
b/src/gallium/include/pipe/p_shader_tokens.h
> index 65d8ad9..5ef6c30 100644
> --- a/src/gallium/include/pipe/p_shader_tokens.h
> +++ b/src/gallium/include/pipe/p_shader_tokens.h
> @@ -237,15 +237,6 @@ struct tgsi_declaration_array {
>     unsigned Padding : 22;
>  };
>
> -/*
> - * Special resources that don't need to be declared.  They map to the
> - * GLOBAL/LOCAL/PRIVATE/INPUT compute memory spaces.
> - */
> -#define TGSI_RESOURCE_GLOBAL   0x7fff
> -#define TGSI_RESOURCE_LOCAL    0x7ffe
> -#define TGSI_RESOURCE_PRIVATE  0x7ffd
> -#define TGSI_RESOURCE_INPUT    0x7ffc
> -
>  #define TGSI_IMM_FLOAT32   0
>  #define TGSI_IMM_UINT32    1
>  #define TGSI_IMM_INT32     2
> --
> 2.7.2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Possibly Parallel Threads

Search for more possibly parallel threads

Nouveau - Mar 2016 - [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE_ defines

[Nouveau] [PATCH mesa v2 1/3] nouveau: codegen: Disable more old resource handling code

[Nouveau] [PATCH mesa v2 2/3] nouveau: codegen: Do not silently fail in handeLOAD / handleSTORE / handleATOM

[Nouveau] [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE_ defines

[Nouveau] [PATCH mesa v2 1/3] nouveau: codegen: Disable more old resource handling code

[Nouveau] [Mesa-dev] [PATCH mesa v2 3/3] gallium: Remove unused TGSI_RESOURCE_ defines

Possibly Parallel Threads