<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Feb 28, 2018 at 1:35 PM, Rob Clark <span dir="ltr"><<a href="mailto:robdclark@gmail.com" target="_blank">robdclark@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Feb 28, 2018 at 4:18 PM, Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>> wrote:<br>
> On Wed, Feb 28, 2018 at 9:46 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> wrote:<br>
>> I do not think this patch does what you think it does.  The old opcode<br>
>> allowed you to shift any bit size integer by a 32-bit integer.  The new<br>
>> version allows you to shift N bits by N bits.  In particular, you can't<br>
>> shift a 16-bit by a 32-bit value.<br>
>><br>
>> I'm not sure what the best thing is to do here.  Really, the size of src1<br>
>> doesn't really matter as 8-bit is enough to do any shifting needed for a<br>
>> 64-bit src0.  You can always compose with a u2u32 to get any src1 bit size<br>
>> you want.  We picked 32 because it's been the GL default for a long time.<br>
>><br>
><br>
> Well the thing is we ended up with a shift in spirv having two 64 bit<br>
> parameters. Maybe we could just put a convert in it for such cases?<br>
><br>
<br>
</span>yeah, I'd overlooked that this was only changing the 2nd src param..<br>
u2u32 for 2nd src would make sense<br></blockquote><div><br></div><div>For now, that definitely works.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
(arguably we might only want to do that if the 2nd src is 64b, ie. if<br>
2nd src is already 16b or 8b no sense to up-convert it to 32b)<br></blockquote><div><br></div><div>Yeah, in future, it may help to add some shift variants which take a smaller src1.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I'll drop this and make a vtn patch to replace it.<br>
<br>
BR,<br>
-R<br>
<div class="HOEnZb"><div class="h5"><br>
>> On Wed, Feb 28, 2018 at 11:51 AM, Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>> wrote:<br>
>>><br>
>>> From: Karol Herbst <<a href="mailto:kherbst@redhat.com">kherbst@redhat.com</a>><br>
>>><br>
>>> This is a thing for OpenCL kernels.<br>
>>><br>
>>> Signed-off-by: Rob Clark <<a href="mailto:robdclark@gmail.com">robdclark@gmail.com</a>><br>
>>> ---<br>
>>>  src/compiler/nir/nir_opcodes.<wbr>py | 6 +++---<br>
>>>  1 file changed, 3 insertions(+), 3 deletions(-)<br>
>>><br>
>>> diff --git a/src/compiler/nir/nir_<wbr>opcodes.py<br>
>>> b/src/compiler/nir/nir_<wbr>opcodes.py<br>
>>> index 278562b2bd1..c4d2c7805eb 100644<br>
>>> --- a/src/compiler/nir/nir_<wbr>opcodes.py<br>
>>> +++ b/src/compiler/nir/nir_<wbr>opcodes.py<br>
>>> @@ -479,9 +479,9 @@ binop("seq", tfloat32, commutative, "(src0 == src1) ?<br>
>>> 1.0f : 0.0f") # Set on Equ<br>
>>>  binop("sne", tfloat32, commutative, "(src0 != src1) ? 1.0f : 0.0f") # Set<br>
>>> on Not Equal<br>
>>><br>
>>><br>
>>> -opcode("ishl", 0, tint, [0, 0], [tint, tuint32], "", "src0 << src1")<br>
>>> -opcode("ishr", 0, tint, [0, 0], [tint, tuint32], "", "src0 >> src1")<br>
>>> -opcode("ushr", 0, tuint, [0, 0], [tuint, tuint32], "", "src0 >> src1")<br>
>>> +opcode("ishl", 0, tint, [0, 0], [tint, tuint], "", "src0 << src1")<br>
>>> +opcode("ishr", 0, tint, [0, 0], [tint, tuint], "", "src0 >> src1")<br>
>>> +opcode("ushr", 0, tuint, [0, 0], [tuint, tuint], "", "src0 >> src1")<br>
>>><br>
>>>  # bitwise logic operators<br>
>>>  #<br>
>>> --<br>
>>> 2.14.3<br>
>>><br>
>><br>
</div></div></blockquote></div><br></div></div>