On 31/03/16 17:16, Connor Abbott wrote:
> This seems a little ugly to me. Since these ops are just moving bits
> around, maybe we should just convert the float64 src/dest to uint64?
> We do this for other opcodes that just shuffle bits around.
>
Yes, this is a possibility. I wonder if the change float64 -> uint64
should be done only to these unpack_double_split_* ops or apply it to
all (un)pack_double*'s src/dst to keep consistency between them.
I executed a piglit run with this change applied only to
unpack_double_split_* and then to all (un)pack_double* ops, none of them
produce regressions.
I don't have a strong opinion here, we can keep the code as it is or use
uint64 instead but I prefer to apply it to all of *pack_double* ops to
keep consistency. What do you think?
Sam
> On Mon, Mar 21, 2016 at 8:06 AM, Samuel Iglesias Gonsálvez
> <sigles...@igalia.com> wrote:
>> From: Iago Toral Quiroga <ito...@igalia.com>
>>
>> Some special alu operations cannot absorve src modifiers safely. For example,
>> double unpack operations split a double into separate 32-bit chunks that
>> are to be manipulated at bit level. If we allow these operations to absorve
>> the srcmod we run into two issues:
>>
>> 1. We would be applying the source modifiers to both 32-bit chunks, which is
>> not correct.
>> 2. If the 32-bit chunks are handled as unsigned data by the driver, the
>> the effect of the source modifier could be lost.
>>
>> So just skip srcmod lowering for these operations.
>> ---
>> src/compiler/nir/nir_lower_to_source_mods.c | 22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/src/compiler/nir/nir_lower_to_source_mods.c
>> b/src/compiler/nir/nir_lower_to_source_mods.c
>> index 1e8c3c2..8964dc0 100644
>> --- a/src/compiler/nir/nir_lower_to_source_mods.c
>> +++ b/src/compiler/nir/nir_lower_to_source_mods.c
>> @@ -33,6 +33,26 @@
>> * easier to not have them when we're doing optimizations.
>> */
>>
>> +/*
>> + * Some special alu operations cannot absorve src modifiers safely. For
>> example,
>> + * double unpack operations split a double into separate 32-bit chunks that
>> + * are to be manipulated at bit level. If we allow these operations to
>> absorve
>> + * the srcmod we run into two issues:
>> + *
>> + * 1. We would be applying the source modifiers to both 32-bit chunks,
>> which is
>> + * not correct.
>> + * 2. If the 32-bit chunks are handled as unsigned data by the driver, the
>> + * the effect of the source modifier could be lost.
>> + *
>> + * So just skip srcmod lowering for these operations.
>> + */
>> +static bool
>> +nir_alu_op_unsafe_for_srcmods(unsigned opcode)
>> +{
>> + return opcode == nir_op_unpack_double_2x32_split_x ||
>> + opcode == nir_op_unpack_double_2x32_split_y;
>> +}
>> +
>> static bool
>> nir_lower_to_source_mods_block(nir_block *block, void *state)
>> {
>> @@ -41,6 +61,8 @@ nir_lower_to_source_mods_block(nir_block *block, void
>> *state)
>> continue;
>>
>> nir_alu_instr *alu = nir_instr_as_alu(instr);
>> + if (nir_alu_op_unsafe_for_srcmods(alu->op))
>> + continue;
>>
>> for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) {
>> if (!alu->src[i].src.is_ssa)
>> --
>> 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
