[Mesa-dev,2/2] glsl: Improve debug output and variable names for opt_dead_code_local.

Submitted by Eric Anholt on March 6, 2014, 7:40 a.m.

Details

Message ID 1394091640-16119-2-git-send-email-eric@anholt.net
State New
Headers show

Not browsing as part of any series.

Commit Message

Eric Anholt March 6, 2014, 7:40 a.m.
I know this code has confused others, and it confused me 3 years later,
too.
---
 src/glsl/opt_dead_code_local.cpp | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/glsl/opt_dead_code_local.cpp b/src/glsl/opt_dead_code_local.cpp
index e7d46ed..703613b 100644
--- a/src/glsl/opt_dead_code_local.cpp
+++ b/src/glsl/opt_dead_code_local.cpp
@@ -38,7 +38,7 @@ 
 #include "ir_optimization.h"
 #include "glsl_types.h"
 
-static bool debug = false;
+static bool debug = true;
 
 namespace {
 
@@ -51,14 +51,14 @@  public:
       assert(ir);
       this->lhs = lhs;
       this->ir = ir;
-      this->available = ir->write_mask;
+      this->unused = ir->write_mask;
    }
 
    ir_variable *lhs;
    ir_assignment *ir;
 
    /* bitmask of xyzw channels written that haven't been used so far. */
-   int available;
+   int unused;
 };
 
 class kill_for_derefs_visitor : public ir_hierarchical_visitor {
@@ -68,7 +68,7 @@  public:
       this->assignments = assignments;
    }
 
-   void kill_channels(ir_variable *const var, int used)
+   void use_channels(ir_variable *const var, int used)
    {
       foreach_list_safe(n, this->assignments) {
 	 assignment_entry *entry = (assignment_entry *) n;
@@ -76,14 +76,14 @@  public:
 	 if (entry->lhs == var) {
 	    if (var->type->is_scalar() || var->type->is_vector()) {
 	       if (debug)
-		  printf("kill %s (0x%01x - 0x%01x)\n", entry->lhs->name,
-			 entry->available, used);
-	       entry->available &= ~used;
-	       if (!entry->available)
+		  printf("used %s (0x%01x - 0x%01x)\n", entry->lhs->name,
+			 entry->unused, used & 0xf);
+	       entry->unused &= ~used;
+	       if (!entry->unused)
 		  entry->remove();
 	    } else {
 	       if (debug)
-		  printf("kill %s\n", entry->lhs->name);
+		  printf("used %s\n", entry->lhs->name);
 	       entry->remove();
 	    }
 	 }
@@ -92,7 +92,7 @@  public:
 
    virtual ir_visitor_status visit(ir_dereference_variable *ir)
    {
-      kill_channels(ir->var, ~0);
+      use_channels(ir->var, ~0);
 
       return visit_continue;
    }
@@ -109,7 +109,7 @@  public:
       used |= 1 << ir->mask.z;
       used |= 1 << ir->mask.w;
 
-      kill_channels(deref->var, used);
+      use_channels(deref->var, used);
 
       return visit_continue_with_parent;
    }
@@ -202,7 +202,7 @@  process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
 	    if (entry->lhs != var)
 	       continue;
 
-	    int remove = entry->available & ir->write_mask;
+	    int remove = entry->unused & ir->write_mask;
 	    if (debug) {
 	       printf("%s 0x%01x - 0x%01x = 0x%01x\n",
 		      var->name,
@@ -219,7 +219,7 @@  process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
 	       }
 
 	       entry->ir->write_mask &= ~remove;
-	       entry->available &= ~remove;
+	       entry->unused &= ~remove;
 	       if (entry->ir->write_mask == 0) {
 		  /* Delete the dead assignment. */
 		  entry->ir->remove();
@@ -283,7 +283,7 @@  process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
       foreach_list(n, assignments) {
 	 assignment_entry *entry = (assignment_entry *) n;
 
-	 printf("    %s (0x%01x)\n", entry->lhs->name, entry->available);
+	 printf("    %s (0x%01x)\n", entry->lhs->name, entry->unused);
       }
    }
 

Comments

>
> -static bool debug = false;
> +static bool debug = true;
>

Accidental debug flag?
Aras Pranckevicius <aras@unity3d.com> writes:

>>
>> -static bool debug = false;
>> +static bool debug = true;
>>
>
> Accidental debug flag?

Yeah, caught that just after sending it out.  I've been bad at these
recently :(
On Wed, Mar 05, 2014 at 11:40:40PM -0800, Eric Anholt wrote:
> I know this code has confused others, and it confused me 3 years later,
> too.
> ---
>  src/glsl/opt_dead_code_local.cpp | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/src/glsl/opt_dead_code_local.cpp b/src/glsl/opt_dead_code_local.cpp
> index e7d46ed..703613b 100644
> --- a/src/glsl/opt_dead_code_local.cpp
> +++ b/src/glsl/opt_dead_code_local.cpp
> @@ -38,7 +38,7 @@
>  #include "ir_optimization.h"
>  #include "glsl_types.h"
>  
> -static bool debug = false;
> +static bool debug = true;
>  
>  namespace {
>  
> @@ -51,14 +51,14 @@ public:
>        assert(ir);
>        this->lhs = lhs;
>        this->ir = ir;
> -      this->available = ir->write_mask;
> +      this->unused = ir->write_mask;
>     }
>  
>     ir_variable *lhs;
>     ir_assignment *ir;
>  
>     /* bitmask of xyzw channels written that haven't been used so far. */
> -   int available;
> +   int unused;
>  };
>  
>  class kill_for_derefs_visitor : public ir_hierarchical_visitor {
> @@ -68,7 +68,7 @@ public:
>        this->assignments = assignments;
>     }
>  
> -   void kill_channels(ir_variable *const var, int used)
> +   void use_channels(ir_variable *const var, int used)
>     {
>        foreach_list_safe(n, this->assignments) {
>  	 assignment_entry *entry = (assignment_entry *) n;
> @@ -76,14 +76,14 @@ public:
>  	 if (entry->lhs == var) {
>  	    if (var->type->is_scalar() || var->type->is_vector()) {
>  	       if (debug)
> -		  printf("kill %s (0x%01x - 0x%01x)\n", entry->lhs->name,
> -			 entry->available, used);
> -	       entry->available &= ~used;
> -	       if (!entry->available)
> +		  printf("used %s (0x%01x - 0x%01x)\n", entry->lhs->name,
> +			 entry->unused, used & 0xf);

There wasn't the masking before, and I started thinking when the caller passes
the higher bits set. I took a a look at the callers, and I believe the reason
is:

  virtual ir_visitor_status visit(ir_dereference_variable *ir)
  {
     kill_channels(ir->var, ~0);


Hence this is:
Reviewed-by: Topi Pohjolainen <topi.pohjolainen@intel.com>

First patch also looks good to me but I really don't know the context well
enough.

> +	       entry->unused &= ~used;
> +	       if (!entry->unused)
>  		  entry->remove();
>  	    } else {
>  	       if (debug)
> -		  printf("kill %s\n", entry->lhs->name);
> +		  printf("used %s\n", entry->lhs->name);
>  	       entry->remove();
>  	    }
>  	 }
> @@ -92,7 +92,7 @@ public:
>  
>     virtual ir_visitor_status visit(ir_dereference_variable *ir)
>     {
> -      kill_channels(ir->var, ~0);
> +      use_channels(ir->var, ~0);
>  
>        return visit_continue;
>     }
> @@ -109,7 +109,7 @@ public:
>        used |= 1 << ir->mask.z;
>        used |= 1 << ir->mask.w;
>  
> -      kill_channels(deref->var, used);
> +      use_channels(deref->var, used);
>  
>        return visit_continue_with_parent;
>     }
> @@ -202,7 +202,7 @@ process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
>  	    if (entry->lhs != var)
>  	       continue;
>  
> -	    int remove = entry->available & ir->write_mask;
> +	    int remove = entry->unused & ir->write_mask;
>  	    if (debug) {
>  	       printf("%s 0x%01x - 0x%01x = 0x%01x\n",
>  		      var->name,
> @@ -219,7 +219,7 @@ process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
>  	       }
>  
>  	       entry->ir->write_mask &= ~remove;
> -	       entry->available &= ~remove;
> +	       entry->unused &= ~remove;
>  	       if (entry->ir->write_mask == 0) {
>  		  /* Delete the dead assignment. */
>  		  entry->ir->remove();
> @@ -283,7 +283,7 @@ process_assignment(void *ctx, ir_assignment *ir, exec_list *assignments)
>        foreach_list(n, assignments) {
>  	 assignment_entry *entry = (assignment_entry *) n;
>  
> -	 printf("    %s (0x%01x)\n", entry->lhs->name, entry->available);
> +	 printf("    %s (0x%01x)\n", entry->lhs->name, entry->unused);
>        }
>     }
>  
> -- 
> 1.9.0
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev