[Mesa-dev] [PATCH 3/5] i965/fs: Remove the requirement of no dead code for interference checks.

[Kenneth Graunke]

On 05/08/2012 05:59 PM, Eric Anholt wrote:
> This will be convenient when I want to comment out optimization code
> to see the raw program being optimized, but more importantly will let
> the interference check be used during optimization.
> ---
>   .../drivers/dri/i965/brw_fs_live_variables.cpp     |   24 ++++++++++----------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> index c7ee582..46408da 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs_live_variables.cpp
> @@ -226,19 +226,19 @@ fs_visitor::calculate_live_intervals()
>   bool
>   fs_visitor::virtual_grf_interferes(int a, int b)
>   {
> -   int start = MAX2(this->virtual_grf_def[a], this->virtual_grf_def[b]);
> -   int end = MIN2(this->virtual_grf_use[a], this->virtual_grf_use[b]);
> -
> -   /* We can't handle dead register writes here, without iterating
> -    * over the whole instruction stream to find every single dead
> -    * write to that register to compare to the live interval of the
> -    * other register.  Just assert that dead_code_eliminate() has been
> -    * called.
> +   int a_def = this->virtual_grf_def[a], a_use = this->virtual_grf_use[a];
> +   int b_def = this->virtual_grf_def[b], b_use = this->virtual_grf_use[b];
> +
> +   /* If there's dead code (def but not use), it would break our test
> +    * unless we consider it used.
>       */

Sorry...I'm not quite sure what test you mean here.  The start < end 
test at the end?  Yes, the dead code case would yield (1 << 30) < -1 and 
fail that test, so we'd return false...aka "it doesn't 
interfere"...which is the opposite of what you've special-cased here.

So if it does need to be marked as interfering, your code is correct.

What are the ramifications of saying that it does or doesn't interfere? 
  (If you'd prefer to just point me at a paper, that's OK too...it's 
been a while since I've really looked at register allocation so I'm 
probably being dense.)

Regardless, I'm glad to see that it's now possible to comment out the 
dead code pass.

Acked-by: Kenneth Graunke <kenneth at whitecape.org>

> -   assert((this->virtual_grf_use[a] != -1 ||
> -	   this->virtual_grf_def[a] == MAX_INSTRUCTION)&&
> -	  (this->virtual_grf_use[b] != -1 ||
> -	   this->virtual_grf_def[b] == MAX_INSTRUCTION));
> +   if ((a_use == -1&&  a_def != MAX_INSTRUCTION) ||
> +       (b_use == -1&&  b_def != MAX_INSTRUCTION)) {
> +      return true;
> +   }
> +
> +   int start = MAX2(a_def, b_def);
> +   int end = MIN2(a_use, b_use);
>
>      /* If the register is used to store 16 values of less than float
>       * size (only the case for pixel_[xy]), then we can't allocate