On 22/03/16 17:37, Jason Ekstrand wrote:
> On Mar 22, 2016 8:18 AM, "Samuel Iglesias Gonsálvez" <sigles...@igalia.com>
> wrote:
>>
>>
>>
>> On 21/03/16 23:54, Jason Ekstrand wrote:
>>> On Mon, Mar 21, 2016 at 5:05 AM, Samuel Iglesias Gonsálvez <
>>> sigles...@igalia.com> wrote:
>>>
>>>> From: Iago Toral Quiroga <ito...@igalia.com>
>>>>
>>>> Undefined sources in alu operations don't have a valid bit size because
>>>> they are uninitialized. Simply ignoring undefined sources for bit size
>>>> validation is not enough since drivers can check and operate with the
>>>> bit-size and that can lead to issues later on. Instead, fix undefined
>>>> sources to always have a compatible bit size.
>>>>
>>>
>>> I'm not sure what I think about this. I think I'd rather have undefs
>>> simply have the right bitsize.
>>>
>>
>> With undefined sources you cannot get the bitsize from themselves
>> because it is not initialized.
>
> Doesn't patch 3 and the discussion on it imply that undefs should have
> valid sizes?
>
Oh right. I will discard this patch as undefs already have valid sizes.
Thanks,
Sam
>> In that case, we pick the bit size from
>> the ALU opcode's input definition. If it is unsized, then we use the
>> destination size.
>
> I dont think pulling the implicit size from the source size is ever
> correct. If you have an explicitly sized source that means it doesn't
> affect and isn't affected by the implicit size.
>
>> I think this is the right bitsize... or am I missing something?
>>
>> Sam
>>
>>>
>>>> v2 (Sam):
>>>> - Use helper to get type size from nir_alu_type.
>>>> ---
>>>> src/compiler/nir/nir_validate.c | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>>
>>>> diff --git a/src/compiler/nir/nir_validate.c
>>>> b/src/compiler/nir/nir_validate.c
>>>> index 9f18d1c..645c15a 100644
>>>> --- a/src/compiler/nir/nir_validate.c
>>>> +++ b/src/compiler/nir/nir_validate.c
>>>> @@ -180,9 +180,11 @@ validate_alu_src(nir_alu_instr *instr, unsigned
>>>> index, validate_state *state)
>>>>
>>>> unsigned num_components;
>>>> unsigned src_bit_size;
>>>> + bool is_undef = false;
>>>> if (src->src.is_ssa) {
>>>> src_bit_size = src->src.ssa->bit_size;
>>>> num_components = src->src.ssa->num_components;
>>>> + is_undef = src->src.ssa->parent_instr->type ==
>>>> nir_instr_type_ssa_undef;
>>>> } else {
>>>> src_bit_size = src->src.reg.reg->bit_size;
>>>> if (src->src.reg.reg->is_packed)
>>>> @@ -205,12 +207,20 @@ validate_alu_src(nir_alu_instr *instr, unsigned
>>>> index, validate_state *state)
>>>>
>>>> if (nir_alu_type_get_type_size(src_type)) {
>>>> /* This source has an explicit bit size */
>>>> + if (is_undef) {
>>>> + src_bit_size = nir_alu_type_get_type_size(src_type);
>>>> + src->src.ssa->bit_size = src_bit_size;
>>>> + }
>>>> assert(nir_alu_type_get_type_size(src_type) == src_bit_size);
>>>> } else {
>>>> if
>>>> (!nir_alu_type_get_type_size(nir_op_infos[instr->op].output_type)) {
>>>> unsigned dest_bit_size =
>>>> instr->dest.dest.is_ssa ? instr->dest.dest.ssa.bit_size
>>>> :
> instr->dest.dest.reg.reg->bit_size;
>>>> + if (is_undef) {
>>>> + src_bit_size = dest_bit_size;
>>>> + src->src.ssa->bit_size = dest_bit_size;
>>>> + }
>>>> assert(dest_bit_size == src_bit_size);
>>>> }
>>>> }
>>>> --
>>>> 2.5.0
>>>>
>>>> _______________________________________________
>>>> mesa-dev mailing list
>>>> mesa-dev@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>>
>>>
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
