[Mesa-dev] [PATCH 2/2] spirv: convert the offset and count operands for bitfield ops to uint32
Samuel Iglesias Gonsálvez
siglesias at igalia.com
Wed May 2 08:12:14 UTC 2018
On 30/04/18 20:51, Jason Ekstrand wrote:
> On Mon, Apr 30, 2018 at 1:46 AM, Samuel Iglesias Gonsálvez
> <siglesias at igalia.com <mailto:siglesias at igalia.com>> wrote:
>
> On 26/04/18 18:14, Jason Ekstrand wrote:
>>
>>
>> On Thu, Apr 26, 2018 at 2:24 AM, Samuel Iglesias Gonsálvez
>> <siglesias at igalia.com <mailto:siglesias at igalia.com>> wrote:
>>
>> SPIR-V allows to define the shift operand for shift opcodes with
>> a bit-size different than 32 bits, but in NIR the opcodes have
>> that limitation. As agreed in the mailing list, this patch adds
>> a conversion to 32 bits to fix this.
>>
>> For more info, see:
>>
>> https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html
>> <https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html>
>>
>> Signed-off-by: Samuel Iglesias Gonsálvez
>> <siglesias at igalia.com <mailto:siglesias at igalia.com>>
>> ---
>> src/compiler/spirv/vtn_alu.c | 48
>> ++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git a/src/compiler/spirv/vtn_alu.c
>> b/src/compiler/spirv/vtn_alu.c
>> index 6f3b82cd5c3..a1654b20273 100644
>> --- a/src/compiler/spirv/vtn_alu.c
>> +++ b/src/compiler/spirv/vtn_alu.c
>> @@ -640,6 +640,54 @@ vtn_handle_alu(struct vtn_builder *b,
>> SpvOp opcode,
>> break;
>> }
>>
>> + case SpvOpBitFieldInsert: {
>> + if (src[2]->bit_size != 32) {
>> + /* Convert the Shift operand to 32 bits, which is
>> the bitsize
>> + * supported by the NIR instruction. See discussion
>> here:
>> + *
>> + *
>> https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html
>> <https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html>
>> + */
>> + src[2] = nir_build_alu(&b->nb, nir_op_u2u32,
>> src[2], NULL, NULL, NULL);
>> + }
>> + if (src[3]->bit_size != 32) {
>> + /* Convert the Shift operand to 32 bits, which is
>> the bitsize
>> + * supported by the NIR instruction. See discussion
>> here:
>> + *
>> + *
>> https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html
>> <https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html>
>> + */
>> + src[3] = nir_build_alu(&b->nb, nir_op_u2u32,
>> src[3], NULL, NULL, NULL);
>> + }
>> + val->ssa->def = nir_build_alu(&b->nb,
>> nir_op_bitfield_insert, src[0], src[1], src[2], src[3]);
>>
>>
>> Just use nir_bitfield_insert here
>>
>>
>> + break;
>> + }
>> +
>> + case SpvOpBitFieldSExtract:
>> + case SpvOpBitFieldUExtract: {
>> + bool swap;
>> + unsigned src_bit_size =
>> glsl_get_bit_size(vtn_src[0]->type);
>> + unsigned dst_bit_size = glsl_get_bit_size(type);
>> + nir_op op = vtn_nir_alu_op_for_spirv_opcode(b, opcode,
>> &swap,
>> +
>> src_bit_size, dst_bit_size);
>> + if (src[1]->bit_size != 32) {
>> + /* Convert the Shift operand to 32 bits, which is
>> the bitsize
>> + * supported by the NIR instruction. See discussion
>> here:
>> + *
>> + *
>> https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html
>> <https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html>
>> + */
>> + src[1] = nir_build_alu(&b->nb, nir_op_u2u32,
>> src[1], NULL, NULL, NULL);
>> + }
>> + if (src[2]->bit_size != 32) {
>> + /* Convert the Shift operand to 32 bits, which is
>> the bitsize
>> + * supported by the NIR instruction. See discussion
>> here:
>> + *
>> + *
>> https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html
>> <https://lists.freedesktop.org/archives/mesa-dev/2018-April/193026.html>
>> + */
>> + src[2] = nir_build_alu(&b->nb, nir_op_u2u32,
>> src[2], NULL, NULL, NULL);
>> + }
>> + val->ssa->def = nir_build_alu(&b->nb, op, src[0],
>> src[1], src[2], src[3]);
>> + break;
>>
>>
>> I'm wondering if we don't want to just do something such as
>>
>> for (unsigned i = 0; i < nir_op_infos[op].num_inputs; i++) {
>> src_bit_size =
>> nir_alu_type_get_get_type_size(nir_op_infos[op].input_types[i]);
>> if (src_bit_size && src_bit_size != src[i]->bit_size) {
>> /* Comment goes here */
>> assert(src_bit_size == 32);
>> assert(op == nir_op_ushr || op == ...);
>> src[i] = nir_u2u32(&b->nb, src[i]);
>> }
>> }
>>
>> And then we can cover all of these cases in one go.
>>
>
> Right, but I don't want to convert all of them. The starting value
> for the loop counter should be 1 for some ops and 2 for others, so
> we don't convert the operand that has the value that we will operate.
>
>
> I believe the sources you don't want to convert will have src_bit_size
> == 0.
>
Right. Just sent a new version of the patch.
Thanks,
Sam
> --Jason
>
>
> I'm going to write a patch for that. Thanks for the suggestions.
>
> Sam
>
>> + }
>> +
>> case SpvOpShiftLeftLogical:
>> case SpvOpShiftRightArithmetic:
>> case SpvOpShiftRightLogical: {
>> --
>> 2.14.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> <mailto:mesa-dev at lists.freedesktop.org>
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>> <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180502/e1dfa968/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180502/e1dfa968/attachment.sig>
More information about the mesa-dev
mailing list