On 11/29/2017 02:17 AM, Samuel Iglesias Gonsálvez wrote:
> On Tue, 2017-11-28 at 13:13 -0800, Ian Romanick wrote:
>> On 11/20/2017 10:25 PM, Samuel Iglesias Gonsálvez wrote:
>>> In that case, nir_eval_const_opcode() will evaluate the conversion
>>> but as it was using destination's bit_size, the resulting
>>> value was just a cast of the source constant value. By passing the
>>> source's bit size, it does the conversion properly.
>>>
>>> Fixes:
>>>
>>> dEQP-VK.spirv_assembly.instruction.*.opspecconstantop.*convert*
>>>
>>> Signed-off-by: Samuel Iglesias Gonsálvez <sigles...@igalia.com>
>>> ---
>>> src/compiler/spirv/spirv_to_nir.c | 31 ++++++++++++++++++++++++++-
>>> ----
>>> 1 file changed, 26 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/src/compiler/spirv/spirv_to_nir.c
>>> b/src/compiler/spirv/spirv_to_nir.c
>>> index 2cc3c275ea9..ffbda4f1946 100644
>>> --- a/src/compiler/spirv/spirv_to_nir.c
>>> +++ b/src/compiler/spirv/spirv_to_nir.c
>>> @@ -1348,14 +1348,35 @@ vtn_handle_constant(struct vtn_builder *b,
>>> SpvOp opcode,
>>> bool swap;
>>> nir_alu_type dst_alu_type =
>>> nir_get_nir_type_for_glsl_type(val->const_type);
>>> nir_alu_type src_alu_type = dst_alu_type;
>>> - nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode,
>>> &swap, src_alu_type, dst_alu_type);
>>> -
>>> unsigned num_components = glsl_get_vector_elements(val-
>>>> const_type);
>>> - unsigned bit_size =
>>> - glsl_get_bit_size(val->const_type);
>>> + unsigned bit_size;
>>>
>>> - nir_const_value src[4];
>>> assert(count <= 7);
>>> +
>>> + switch (opcode) {
>>> + case SpvOpUConvert:
>>> + case SpvOpConvertFToU:
>>> + case SpvOpConvertFToS:
>>> + case SpvOpConvertSToF:
>>> + case SpvOpConvertUToF:
>>> + case SpvOpSConvert:
>>> + case SpvOpFConvert:
>>
>> I'm not sure what we should do here. Most of these opcodes are not
>> valid in SpvOpSpecConstantOp in a Shader. Only SpvOpSConvert and
>> SpvOpFConvert are allowed. I guess it's trivial enough to support
>> the
>> others anyway...
>
> Right.
>
>> do those dEQP-VK.spirv_assembly.instruction.* tests
>> exercise the other opcodes?
>>
>
> No, they only exercise SpvOpFConvert and SpvOpSConvert. Do you prefer
> to just support these two opcodes for the moment?
Okay... good. I wanted to make sure they weren't expecting out-of-spec
behavior. :)
> With or without that change, does it get your R-b?
Yes.
Reviewed-by: Ian Romanick <ian.d.roman...@intel.com>
>
> Sam
>
>>> + /* We have a source in a conversion */
>>> + src_alu_type =
>>> + nir_get_nir_type_for_glsl_type(
>>> + vtn_value(b, w[4], vtn_value_type_constant)-
>>>> const_type);
>>> + /* We use the bitsize of the conversion source to
>>> evaluate the opcode later */
>>> + bit_size = glsl_get_bit_size(
>>> + vtn_value(b, w[4], vtn_value_type_constant)-
>>>> const_type);
>>> + break;
>>> + default:
>>> + bit_size = glsl_get_bit_size(val->const_type);
>>> + };
>>> +
>>> +
>>> + nir_op op = vtn_nir_alu_op_for_spirv_opcode(opcode,
>>> &swap, src_alu_type, dst_alu_type);
>>> + nir_const_value src[4];
>>> +
>>> for (unsigned i = 0; i < count - 4; i++) {
>>> nir_constant *c =
>>> vtn_value(b, w[4 + i], vtn_value_type_constant)-
>>>> constant;
>>>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
