<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 30, 2015 at 5:28 PM, Matt Turner <span dir="ltr"><<a href="mailto:mattst88@gmail.com" target="_blank">mattst88@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Fri, Jan 30, 2015 at 4:02 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
> On Fri, Jan 30, 2015 at 3:54 PM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>
>> ---<br>
>>  src/mesa/drivers/dri/i965/brw_shader.cpp | 37<br>
>> ++++++++++++++++++++++++++++++++<br>
>>  src/mesa/drivers/dri/i965/brw_shader.h   |  1 +<br>
>>  2 files changed, 38 insertions(+)<br>
>><br>
>> diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
>> b/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
>> index c393bfc..ff2edf3 100644<br>
>> --- a/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
>> +++ b/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
>> @@ -620,6 +620,43 @@ brw_saturate_immediate(enum brw_reg_type type, struct<br>
>> brw_reg *reg)<br>
>>     return false;<br>
>>  }<br>
>><br>
>> +bool<br>
>> +brw_negate_immediate(enum brw_reg_type type, struct brw_reg *reg)<br>
>> +{<br>
>> +   switch (type) {<br>
>> +   case BRW_REGISTER_TYPE_UD:<br>
>> +   case BRW_REGISTER_TYPE_D:<br>
>> +      reg->dw1.d = -reg->dw1.d;<br>
>> +      return true;<br>
>> +   case BRW_REGISTER_TYPE_UW:<br>
>> +      reg->dw1.d = -(uint16_t)reg->dw1.ud;<br>
><br>
><br>
> Do we really want to negate UW and UD sources?  If so, shouldn't we change<br>
> the register type to signed or something?  Maybe there's something I don't<br>
> know about the arch, but this seems fishy.<br>
<br>
</div></div>I think it's okay. The source language lets you do it and the hardware<br>
lets you do it. I don't think D vs UD (or W vs UW) changes the<br>
operation the negation modifier performs (maybe except for the case of<br>
using it in conjunction with conditional mod).<br>
<br>
In any case, it doesn't catch anything in shader-db, so I don't mind<br>
just returning false for UD and UW.<br>
<span class=""><br>
> Same comment applies on the next patch with abs().<br>
<br>
</span>Surely an abs source modifier on an unsigned source is a no-op. Again,<br>
no changes in shader-db because of allowing it though, so I don't mind<br>
just returning false.<br></blockquote><div><br></div><div>Right...<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> Other than that, everything in this series looks fine.<br>
<br>
</span>Thanks. I'll slap your R-b on it.<br></blockquote><div><br></div><div>Go ahead. <br></div></div><br></div></div>