[Mesa-dev] [PATCH] nir: Allow to skip integer ops in nir_lower_to_source_mods
Jason Ekstrand
jason at jlekstrand.net
Sun Nov 11 15:20:20 UTC 2018
On November 11, 2018 09:11:26 Gert Wollny <gert.wollny at collabora.com> wrote:
> Am Sonntag, den 11.11.2018, 09:07 -0600 schrieb Jason Ekstrand:
>> On November 11, 2018 07:44:54 Gert Wollny <gw.fossdev at gmail.com>
>> wrote:
>>
>>> From: Gert Wollny <gert.wollny at collabora.com>
>>>
>>>
>>>
>>>
>>> Since some hardware supports source mods only for float operations
>>> make
>>> it possible to skip this lowering for integer operations.
>>
>> Out of curiosity, what hardware would that be?
> r600/Evergreen at least.
>
>>
>>> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
>>> ---
>>> I'm a bit unsure about what the best name for the parameter is,
>>> i.e. passing in
>>> true when one doesn't want all to be lowered seems a bit
>>> irritating, but a name
>>> like "lower_also_int_ops" looks ugly, and "lower_all" immediately
>>> asks for a
>>> comment what "not all" includes. I don't think it's worth adding
>>> option flags
>>> that would be more self-explanatory though.
>>
>> Maybe a bit field:
>>
>> typedef enum {
>> nir_lower_int_source_mods = 1 << 0,
>> nir_lower_float_source_mods = 1 << 1,
>> } nir_lower_to_source_mods_flags;
>
> That was my first idea, but then it seemed a bit too much, i.e. I don't
> know whether there is hardware that can do has source modes for int,
> but not for float.
Meh. It makes it clear and that's the point. I don't really care if it's
"too much" as long as the code remains readable.
As an aside, Intel hardware has source mods for integers but not all
instructions support them and on Broadwell and later, logical instructions
(and, or, xor) treat the negation modifier as doing an inot so it doesn't
map perfectly for us either.
>
> Best,
> Gert
>
>>
>>> thanks for any comments and reviews,
>>> Gert
>>>
>>>
>>>
>>>
>>> src/compiler/nir/nir.h | 2 +-
>>> src/compiler/nir/nir_lower_to_source_mods.c | 36 +++++++++++++-----
>>> ---
>>> src/intel/compiler/brw_nir.c | 2 +-
>>> 3 files changed, 24 insertions(+), 16 deletions(-)
>>>
>>>
>>> diff --git a/src/compiler/nir/nir.h b/src/compiler/nir/nir.h
>>> index dc3c729dee..e2f64c9d44 100644
>>> --- a/src/compiler/nir/nir.h
>>> +++ b/src/compiler/nir/nir.h
>>> @@ -3013,7 +3013,7 @@ typedef struct nir_lower_bitmap_options {
>>> void nir_lower_bitmap(nir_shader *shader, const
>>> nir_lower_bitmap_options
>>> *options);
>>>
>>> bool nir_lower_atomics_to_ssbo(nir_shader *shader, unsigned
>>> ssbo_offset);
>>> -bool nir_lower_to_source_mods(nir_shader *shader);
>>> +bool nir_lower_to_source_mods(nir_shader *shader, bool
>>> skip_int_ops);
>>>
>>> bool nir_lower_gs_intrinsics(nir_shader *shader);
>>>
>>> diff --git a/src/compiler/nir/nir_lower_to_source_mods.c
>>> b/src/compiler/nir/nir_lower_to_source_mods.c
>>> index 077ca53704..36f2160627 100644
>>> --- a/src/compiler/nir/nir_lower_to_source_mods.c
>>> +++ b/src/compiler/nir/nir_lower_to_source_mods.c
>>> @@ -34,7 +34,7 @@
>>> */
>>>
>>> static bool
>>> -nir_lower_to_source_mods_block(nir_block *block)
>>> +nir_lower_to_source_mods_block(nir_block *block, bool
>>> skip_int_ops)
>>> {
>>> bool progress = false;
>>>
>>> @@ -62,6 +62,8 @@ nir_lower_to_source_mods_block(nir_block *block)
>>> continue;
>>> break;
>>> case nir_type_int:
>>> + if (skip_int_ops)
>>> + continue;
>>> if (parent->op != nir_op_imov)
>>> continue;
>>> break;
>>> @@ -102,19 +104,10 @@ nir_lower_to_source_mods_block(nir_block
>>> *block)
>>> alu->op = nir_op_fmov;
>>> alu->dest.saturate = true;
>>> break;
>>> - case nir_op_ineg:
>>> - alu->op = nir_op_imov;
>>> - alu->src[0].negate = !alu->src[0].negate;
>>> - break;
>>> case nir_op_fneg:
>>> alu->op = nir_op_fmov;
>>> alu->src[0].negate = !alu->src[0].negate;
>>> break;
>>> - case nir_op_iabs:
>>> - alu->op = nir_op_imov;
>>> - alu->src[0].abs = true;
>>> - alu->src[0].negate = false;
>>> - break;
>>> case nir_op_fabs:
>>> alu->op = nir_op_fmov;
>>> alu->src[0].abs = true;
>>> @@ -124,6 +117,21 @@ nir_lower_to_source_mods_block(nir_block
>>> *block)
>>> break;
>>> }
>>>
>>> + if (!skip_int_ops) {
>>> + switch (alu->op) {
>>> + case nir_op_ineg:
>>> + alu->op = nir_op_imov;
>>> + alu->src[0].negate = !alu->src[0].negate;
>>> + break;
>>> + case nir_op_iabs:
>>> + alu->op = nir_op_imov;
>>> + alu->src[0].abs = true;
>>> + alu->src[0].negate = false;
>>> + break;
>>> + default:
>>> + break;
>>> + }
>>> + }
>>> /* We've covered sources. Now we're going to try and
>>> saturate the
>>> * destination if we can.
>>> */
>>> @@ -185,12 +193,12 @@ nir_lower_to_source_mods_block(nir_block
>>> *block)
>>> }
>>>
>>> static bool
>>> -nir_lower_to_source_mods_impl(nir_function_impl *impl)
>>> +nir_lower_to_source_mods_impl(nir_function_impl *impl, bool
>>> skip_int_ops)
>>> {
>>> bool progress = false;
>>>
>>> nir_foreach_block(block, impl) {
>>> - progress |= nir_lower_to_source_mods_block(block);
>>> + progress |= nir_lower_to_source_mods_block(block,
>>> skip_int_ops);
>>> }
>>>
>>> if (progress)
>>> @@ -201,13 +209,13 @@
>>> nir_lower_to_source_mods_impl(nir_function_impl *impl)
>>> }
>>>
>>> bool
>>> -nir_lower_to_source_mods(nir_shader *shader)
>>> +nir_lower_to_source_mods(nir_shader *shader, bool skip_int_ops)
>>> {
>>> bool progress = false;
>>>
>>> nir_foreach_function(function, shader) {
>>> if (function->impl) {
>>> - progress |= nir_lower_to_source_mods_impl(function-
>>>> impl);
>>> + progress |= nir_lower_to_source_mods_impl(function-
>>>> impl,
>>> skip_int_ops);
>>> }
>>> }
>>>
>>> diff --git a/src/intel/compiler/brw_nir.c
>>> b/src/intel/compiler/brw_nir.c
>>> index cf5a4a96d6..9c5f2af030 100644
>>> --- a/src/intel/compiler/brw_nir.c
>>> +++ b/src/intel/compiler/brw_nir.c
>>> @@ -793,7 +793,7 @@ brw_postprocess_nir(nir_shader *nir, const
>>> struct
>>> brw_compiler *compiler,
>>>
>>> OPT(nir_opt_algebraic_late);
>>>
>>> - OPT(nir_lower_to_source_mods);
>>> + OPT(nir_lower_to_source_mods, false);
>>> OPT(nir_copy_prop);
>>> OPT(nir_opt_dce);
>>> OPT(nir_opt_move_comparisons);
>>> --
>>> 2.18.1
>>
>>
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list