On Fri, Apr 1, 2016 at 4:56 AM, Samuel Iglesias Gonsálvez
<sigles...@igalia.com> wrote:
> 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?
Yes, we should change all of the pack/unpack double ops. There could
theoretically be an issue with the pack ops if we put a saturate
modifier on the dest too, which this would fix as well.
>
> 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
