Re: [Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
On Mon, Jan 21, 2019 at 2:09 AM Iago Toral wrote:
> On Fri, 2019-01-18 at 06:46 -0600, Jason Ekstrand wrote:
>
> On January 18, 2019 01:48:25 Iago Toral wrote:
>
> On Thu, 2019-01-17 at 13:42 -0600, Jason Ekstrand wrote:
>
> On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga
> wrote
>
>
> (...)
>
>
> + nir_ssa_def *tmp = nir_build_alu(b, op1, src, NULL, NULL, NULL);
> + nir_ssa_def *res = nir_build_alu(b, op2, tmp, NULL, NULL, NULL);
> + nir_ssa_def_rewrite_uses(>dest.dest.ssa, nir_src_for_ssa(res));
> + nir_instr_remove(>instr);
> +}
> +
> +static bool
> +lower_instr(nir_builder *b, nir_alu_instr *alu)
> +{
> + unsigned src_bit_size = nir_src_bit_size(alu->src[0].src);
> + nir_alu_type src_type = nir_op_infos[alu->op].input_types[0];
> + nir_alu_type src_full_type = (nir_alu_type) (src_type | src_bit_size);
> +
> + unsigned dst_bit_size = nir_dest_bit_size(alu->dest.dest);
> + nir_alu_type dst_type = nir_op_infos[alu->op].output_type;
> + nir_alu_type dst_full_type = (nir_alu_type) (dst_type | dst_bit_size);
>
>
> The nir_op_infos[*].output_type will already be a full type. Maybe just
> delete the dst_type temporary so that no one gets confused and thinks it's
> a base type.
>
>
> Right, will do that.
>
>
> Actually, we want the base type too, since that is what we pass to the
> get_conversion_op() helper, I'll just pick it from the dst_full_type with
> the nir_alu_type_get_base_type() helper.
>
Sounds like the right thing to do.
> +
> + /* BDW PRM, vol02, Command Reference Instructions, mov - MOVE:
> +*
> +* "There is no direct conversion from HF to DF or DF to HF.
> +*Use two instructions and F (Float) as an intermediate type.
> +*
> +*There is no direct conversion from HF to Q/UQ or Q/UQ to HF.
> +*Use two instructions and F (Float) or a word integer type
> +*or a DWord integer type as an intermediate type."
> +*/
> + if ((src_full_type == nir_type_float16 && dst_bit_size == 64) ||
> + (src_bit_size == 64 && dst_full_type == nir_type_float16)) {
> + nir_op op1 = get_conversion_op(src_type, src_bit_size, src_type, 32,
> + nir_rounding_mode_undef);
> + nir_op op2 = get_conversion_op(src_type, 32, dst_type, dst_bit_size,
> + get_opcode_rounding_mode(alu->op));
> + split_conversion(b, alu, op1, op2);
> + return true;
> + }
> +
> + /* SKL PRM, vol 02a, Command Reference: Instructions, Move:
> +*
> +* "There is no direct conversion from B/UB to DF or DF to B/UB. Use
> +*two instructions and a word or DWord intermediate type."
> +*
> +* "There is no direct conversion from B/UB to Q/UQ or Q/UQ to B/UB.
> +*Use two instructions and a word or DWord intermediate integer
> +*type."
> +*/
> + if ((src_bit_size == 8 && dst_bit_size == 64) ||
> + (src_bit_size == 64 && dst_bit_size == 8)) {
> + nir_op op1 = get_conversion_op(src_type, src_bit_size, src_type, 32,
> + nir_rounding_mode_undef);
> + nir_op op2 = get_conversion_op(src_type, 32, dst_type, dst_bit_size,
> + nir_rounding_mode_undef);
> + split_conversion(b, alu, op1, op2);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static bool
> +lower_impl(nir_function_impl *impl)
> +{
> + nir_builder b;
> + nir_builder_init(, impl);
> + bool progress = false;
> +
> + nir_foreach_block(block, impl) {
> + nir_foreach_instr_safe(instr, block) {
> + if (instr->type != nir_instr_type_alu)
> +continue;
> +
> + nir_alu_instr *alu = nir_instr_as_alu(instr);
> + assert(alu->dest.dest.is_ssa);
> +
> + if (nir_op_infos[alu->op].num_inputs > 1)
>
>
> I don't think this is a reliable way to detect conversion opcodes. There
> are many unary operations that aren't conversions. You're only getting
> lucky because there are non which do float16 <-> 64=bit or 8-bit <->
> 64-bit. I've thought many times about throwing a boolean in nir_op_infos
> for is_conversion. Maybe now is a good time to do it?
>
>
> Yes, I knew that this was not going to only let conversions through and
> was relying on what you say above. Adding an is_conversion field souns good
> to me, alternatively I guess we could add a is_conversion() helper, but I
> guess having the field in nir_op_infos maye be a bit easier to maintain if
> we want to add more opcodes in the future.
>
>
> Thinking about this, I guess we only want to set this to True for the
> opcodes that are explicit numeric conversions, that is, the ones we
> generate right after "# Generate all of the numeric conversion opcodes". I
> am saying this because even if we have unop_convert(), that is also used
> for things like unpack opcodes, but_count, etc that are not real
> conversions. I was thinking about creating a new unop_numeric_convert()
> helper
Re: [Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
On Fri, 2019-01-18 at 06:46 -0600, Jason Ekstrand wrote:
> On January 18, 2019 01:48:25 Iago Toral wrote:
>
> > On Thu, 2019-01-17 at 13:42 -0600, Jason Ekstrand wrote:
> > > On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga <
> > > ito...@igalia.com> wrote
(...)
> > > > + nir_ssa_def *tmp = nir_build_alu(b, op1, src, NULL, NULL,
> > > > NULL);
> > > >
> > > > + nir_ssa_def *res = nir_build_alu(b, op2, tmp, NULL, NULL,
> > > > NULL);
> > > >
> > > > + nir_ssa_def_rewrite_uses(>dest.dest.ssa,
> > > > nir_src_for_ssa(res));
> > > >
> > > > + nir_instr_remove(>instr);
> > > >
> > > > +}
> > > >
> > > > +
> > > >
> > > > +static bool
> > > >
> > > > +lower_instr(nir_builder *b, nir_alu_instr *alu)
> > > >
> > > > +{
> > > >
> > > > + unsigned src_bit_size = nir_src_bit_size(alu->src[0].src);
> > > >
> > > > + nir_alu_type src_type = nir_op_infos[alu-
> > > > >op].input_types[0];
> > > >
> > > > + nir_alu_type src_full_type = (nir_alu_type) (src_type |
> > > > src_bit_size);
> > > >
> > > > +
> > > >
> > > > + unsigned dst_bit_size = nir_dest_bit_size(alu->dest.dest);
> > > >
> > > > + nir_alu_type dst_type = nir_op_infos[alu->op].output_type;
> > > >
> > > > + nir_alu_type dst_full_type = (nir_alu_type) (dst_type |
> > > > dst_bit_size);
> > >
> > > The nir_op_infos[*].output_type will already be a full type.
> > > Maybe just delete the dst_type temporary so that no one gets
> > > confused and thinks it's a base type.
> >
> > Right, will do that.
Actually, we want the base type too, since that is what we pass to the
get_conversion_op() helper, I'll just pick it from the dst_full_type
with the nir_alu_type_get_base_type() helper.
> > > > +
> > > >
> > > > + /* BDW PRM, vol02, Command Reference Instructions, mov -
> > > > MOVE:
> > > >
> > > > +*
> > > >
> > > > +* "There is no direct conversion from HF to DF or DF to
> > > > HF.
> > > >
> > > > +*Use two instructions and F (Float) as an intermediate
> > > > type.
> > > >
> > > > +*
> > > >
> > > > +*There is no direct conversion from HF to Q/UQ or Q/UQ
> > > > to HF.
> > > >
> > > > +*Use two instructions and F (Float) or a word integer
> > > > type
> > > >
> > > > +*or a DWord integer type as an intermediate type."
> > > >
> > > > +*/
> > > >
> > > > + if ((src_full_type == nir_type_float16 && dst_bit_size ==
> > > > 64) ||
> > > >
> > > > + (src_bit_size == 64 && dst_full_type ==
> > > > nir_type_float16)) {
> > > >
> > > > + nir_op op1 = get_conversion_op(src_type, src_bit_size,
> > > > src_type, 32,
> > > >
> > > > + nir_rounding_mode_undef);
> > > >
> > > > + nir_op op2 = get_conversion_op(src_type, 32, dst_type,
> > > > dst_bit_size,
> > > >
> > > > +
> > > > get_opcode_rounding_mode(alu->op));
> > > >
> > > > + split_conversion(b, alu, op1, op2);
> > > >
> > > > + return true;
> > > >
> > > > + }
> > > >
> > > > +
> > > >
> > > > + /* SKL PRM, vol 02a, Command Reference: Instructions, Move:
> > > >
> > > > +*
> > > >
> > > > +* "There is no direct conversion from B/UB to DF or DF
> > > > to B/UB. Use
> > > >
> > > > +*two instructions and a word or DWord intermediate
> > > > type."
> > > >
> > > > +*
> > > >
> > > > +* "There is no direct conversion from B/UB to Q/UQ or
> > > > Q/UQ to B/UB.
> > > >
> > > > +*Use two instructions and a word or DWord intermediate
> > > > integer
> > > >
> > > > +*type."
> > > >
> > > > +*/
> > > >
> > > > + if ((src_bit_size == 8 && dst_bit_size == 64) ||
> > > >
> > > > + (src_bit_size == 64 && dst_bit_size == 8)) {
> > > >
> > > > + nir_op op1 = get_conversion_op(src_type, src_bit_size,
> > > > src_type, 32,
> > > >
> > > > + nir_rounding_mode_undef);
> > > >
> > > > + nir_op op2 = get_conversion_op(src_type, 32, dst_type,
> > > > dst_bit_size,
> > > >
> > > > + nir_rounding_mode_undef);
> > > >
> > > > + split_conversion(b, alu, op1, op2);
> > > >
> > > > + return true;
> > > >
> > > > + }
> > > >
> > > > +
> > > >
> > > > + return false;
> > > >
> > > > +}
> > > >
> > > > +
> > > >
> > > > +static bool
> > > >
> > > > +lower_impl(nir_function_impl *impl)
> > > >
> > > > +{
> > > >
> > > > + nir_builder b;
> > > >
> > > > + nir_builder_init(, impl);
> > > >
> > > > + bool progress = false;
> > > >
> > > > +
> > > >
> > > > + nir_foreach_block(block, impl) {
> > > >
> > > > + nir_foreach_instr_safe(instr, block) {
> > > >
> > > > + if (instr->type != nir_instr_type_alu)
> > > >
> > > > +continue;
> > > >
> > > > +
> > > >
> > > > + nir_alu_instr *alu = nir_instr_as_alu(instr);
> > > >
> > > > + assert(alu->dest.dest.is_ssa);
> > > >
> > > > +
> > > >
> > > > + if
Re: [Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
On January 18, 2019 01:48:25 Iago Toral wrote:
On Thu, 2019-01-17 at 13:42 -0600, Jason Ekstrand wrote:
On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga wrote:
Some conversions are not directly supported in hardware and need to be
split in two conversion instructions going through an intermediary type.
Doing this at the NIR level simplifies a bit the complexity in the backend.
v2:
- Consider fp16 rounding conversion opcodes
- Properly handle swizzles on conversion sources.
Reviewed-by: Topi Pohjolainen (v1)
---
src/intel/Makefile.sources| 1 +
src/intel/compiler/brw_nir.c | 1 +
src/intel/compiler/brw_nir.h | 2 +
.../compiler/brw_nir_lower_conversions.c | 158 ++
src/intel/compiler/meson.build| 1 +
5 files changed, 163 insertions(+)
create mode 100644 src/intel/compiler/brw_nir_lower_conversions.c
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 94a28d370e8..9975daa3ad1 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -83,6 +83,7 @@ COMPILER_FILES = \
compiler/brw_nir_analyze_boolean_resolves.c \
compiler/brw_nir_analyze_ubo_ranges.c \
compiler/brw_nir_attribute_workarounds.c \
+ compiler/brw_nir_lower_conversions.c \
compiler/brw_nir_lower_cs_intrinsics.c \
compiler/brw_nir_lower_image_load_store.c \
compiler/brw_nir_lower_mem_access_bit_sizes.c \
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 92d7fe4bede..572ab824a94 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -882,6 +882,7 @@ brw_postprocess_nir(nir_shader *nir, const struct
brw_compiler *compiler,
OPT(nir_opt_move_comparisons);
OPT(nir_lower_bool_to_int32);
+ OPT(brw_nir_lower_conversions);
This is *really* late and I don't think you actually want this to run after
we apply source/destination modifiers. If you just move it to right after
nir_opt_algebraic_late, that will fix a multitude of issues.
Sure, I'll move it there.
OPT(nir_lower_locals_to_regs);
diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
index bc81950d47e..662b2627e95 100644
--- a/src/intel/compiler/brw_nir.h
+++ b/src/intel/compiler/brw_nir.h
@@ -114,6 +114,8 @@ void brw_nir_lower_tcs_outputs(nir_shader *nir, const
struct brw_vue_map *vue,
GLenum tes_primitive_mode);
void brw_nir_lower_fs_outputs(nir_shader *nir);
+bool brw_nir_lower_conversions(nir_shader *nir);
+
bool brw_nir_lower_image_load_store(nir_shader *nir,
const struct gen_device_info *devinfo);
void brw_nir_rewrite_image_intrinsic(nir_intrinsic_instr *intrin,
diff --git a/src/intel/compiler/brw_nir_lower_conversions.c
b/src/intel/compiler/brw_nir_lower_conversions.c
new file mode 100644
index 000..583167c7753
--- /dev/null
+++ b/src/intel/compiler/brw_nir_lower_conversions.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "brw_nir.h"
+#include "compiler/nir/nir_builder.h"
+
+static nir_op
+get_conversion_op(nir_alu_type src_type,
+ unsigned src_bit_size,
+ nir_alu_type dst_type,
+ unsigned dst_bit_size,
+ nir_rounding_mode rounding_mode)
+{
+ nir_alu_type src_full_type = (nir_alu_type) (src_type | src_bit_size);
+ nir_alu_type dst_full_type = (nir_alu_type) (dst_type | dst_bit_size);
+
+ return nir_type_conversion_op(src_full_type, dst_full_type, rounding_mode);
+}
+
+static nir_rounding_mode
+get_opcode_rounding_mode(nir_op op)
+{
+ switch (op) {
+ case nir_op_f2f16_rtz:
+ return nir_rounding_mode_rtz;
+ case nir_op_f2f16_rtne:
+ return nir_rounding_mode_rtne;
+ default:
+ return
Re: [Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
On Thu, 2019-01-17 at 13:42 -0600, Jason Ekstrand wrote: > On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga > wrote: > > Some conversions are not directly supported in hardware and need to > > be > > > > split in two conversion instructions going through an intermediary > > type. > > > > Doing this at the NIR level simplifies a bit the complexity in the > > backend. > > > > > > > > v2: > > > > - Consider fp16 rounding conversion opcodes > > > > - Properly handle swizzles on conversion sources. > > > > > > > > Reviewed-by: Topi Pohjolainen (v1) > > > > --- > > > > src/intel/Makefile.sources| 1 + > > > > src/intel/compiler/brw_nir.c | 1 + > > > > src/intel/compiler/brw_nir.h | 2 + > > > > .../compiler/brw_nir_lower_conversions.c | 158 > > ++ > > > > src/intel/compiler/meson.build| 1 + > > > > 5 files changed, 163 insertions(+) > > > > create mode 100644 src/intel/compiler/brw_nir_lower_conversions.c > > > > > > > > diff --git a/src/intel/Makefile.sources > > b/src/intel/Makefile.sources > > > > index 94a28d370e8..9975daa3ad1 100644 > > > > --- a/src/intel/Makefile.sources > > > > +++ b/src/intel/Makefile.sources > > > > @@ -83,6 +83,7 @@ COMPILER_FILES = \ > > > > compiler/brw_nir_analyze_boolean_resolves.c \ > > > > compiler/brw_nir_analyze_ubo_ranges.c \ > > > > compiler/brw_nir_attribute_workarounds.c \ > > > > + compiler/brw_nir_lower_conversions.c \ > > > > compiler/brw_nir_lower_cs_intrinsics.c \ > > > > compiler/brw_nir_lower_image_load_store.c \ > > > > compiler/brw_nir_lower_mem_access_bit_sizes.c \ > > > > diff --git a/src/intel/compiler/brw_nir.c > > b/src/intel/compiler/brw_nir.c > > > > index 92d7fe4bede..572ab824a94 100644 > > > > --- a/src/intel/compiler/brw_nir.c > > > > +++ b/src/intel/compiler/brw_nir.c > > > > @@ -882,6 +882,7 @@ brw_postprocess_nir(nir_shader *nir, const > > struct brw_compiler *compiler, > > > > OPT(nir_opt_move_comparisons); > > > > > > > > OPT(nir_lower_bool_to_int32); > > > > + OPT(brw_nir_lower_conversions); > > This is *really* late and I don't think you actually want this to run > after we apply source/destination modifiers. If you just move it to > right after nir_opt_algebraic_late, that will fix a multitude of > issues. Sure, I'll move it there. > > > > OPT(nir_lower_locals_to_regs); > > > > > > > > diff --git a/src/intel/compiler/brw_nir.h > > b/src/intel/compiler/brw_nir.h > > > > index bc81950d47e..662b2627e95 100644 > > > > --- a/src/intel/compiler/brw_nir.h > > > > +++ b/src/intel/compiler/brw_nir.h > > > > @@ -114,6 +114,8 @@ void brw_nir_lower_tcs_outputs(nir_shader *nir, > > const struct brw_vue_map *vue, > > > > GLenum tes_primitive_mode); > > > > void brw_nir_lower_fs_outputs(nir_shader *nir); > > > > > > > > +bool brw_nir_lower_conversions(nir_shader *nir); > > > > + > > > > bool brw_nir_lower_image_load_store(nir_shader *nir, > > > > const struct gen_device_info > > *devinfo); > > > > void brw_nir_rewrite_image_intrinsic(nir_intrinsic_instr *intrin, > > > > diff --git a/src/intel/compiler/brw_nir_lower_conversions.c > > b/src/intel/compiler/brw_nir_lower_conversions.c > > > > new file mode 100644 > > > > index 000..583167c7753 > > > > --- /dev/null > > > > +++ b/src/intel/compiler/brw_nir_lower_conversions.c > > > > @@ -0,0 +1,158 @@ > > > > +/* > > > > + * Copyright © 2018 Intel Corporation > > > > + * > > > > + * Permission is hereby granted, free of charge, to any person > > obtaining a > > > > + * copy of this software and associated documentation files (the > > "Software"), > > > > + * to deal in the Software without restriction, including without > > limitation > > > > + * the rights to use, copy, modify, merge, publish, distribute, > > sublicense, > > > > + * and/or sell copies of the Software, and to permit persons to > > whom the > > > > + * Software is furnished to do so, subject to the following > > conditions: > > > > + * > > > > + * The above copyright notice and this permission notice > > (including the next > > > > + * paragraph) shall be included in all copies or substantial > > portions of the > > > > + * Software. > > > > + * > > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > EXPRESS OR > > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > MERCHANTABILITY, > > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > > EVENT SHALL > > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > DAMAGES OR OTHER > > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > ARISING > > > > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > OTHER DEALINGS > > > > + * IN THE SOFTWARE. > >
Re: [Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
On Tue, Jan 15, 2019 at 7:54 AM Iago Toral Quiroga
wrote:
> Some conversions are not directly supported in hardware and need to be
> split in two conversion instructions going through an intermediary type.
> Doing this at the NIR level simplifies a bit the complexity in the backend.
>
> v2:
> - Consider fp16 rounding conversion opcodes
> - Properly handle swizzles on conversion sources.
>
> Reviewed-by: Topi Pohjolainen (v1)
> ---
> src/intel/Makefile.sources| 1 +
> src/intel/compiler/brw_nir.c | 1 +
> src/intel/compiler/brw_nir.h | 2 +
> .../compiler/brw_nir_lower_conversions.c | 158 ++
> src/intel/compiler/meson.build| 1 +
> 5 files changed, 163 insertions(+)
> create mode 100644 src/intel/compiler/brw_nir_lower_conversions.c
>
> diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
> index 94a28d370e8..9975daa3ad1 100644
> --- a/src/intel/Makefile.sources
> +++ b/src/intel/Makefile.sources
> @@ -83,6 +83,7 @@ COMPILER_FILES = \
> compiler/brw_nir_analyze_boolean_resolves.c \
> compiler/brw_nir_analyze_ubo_ranges.c \
> compiler/brw_nir_attribute_workarounds.c \
> + compiler/brw_nir_lower_conversions.c \
> compiler/brw_nir_lower_cs_intrinsics.c \
> compiler/brw_nir_lower_image_load_store.c \
> compiler/brw_nir_lower_mem_access_bit_sizes.c \
> diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
> index 92d7fe4bede..572ab824a94 100644
> --- a/src/intel/compiler/brw_nir.c
> +++ b/src/intel/compiler/brw_nir.c
> @@ -882,6 +882,7 @@ brw_postprocess_nir(nir_shader *nir, const struct
> brw_compiler *compiler,
> OPT(nir_opt_move_comparisons);
>
> OPT(nir_lower_bool_to_int32);
> + OPT(brw_nir_lower_conversions);
>
This is *really* late and I don't think you actually want this to run after
we apply source/destination modifiers. If you just move it to right after
nir_opt_algebraic_late, that will fix a multitude of issues.
>
> OPT(nir_lower_locals_to_regs);
>
> diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
> index bc81950d47e..662b2627e95 100644
> --- a/src/intel/compiler/brw_nir.h
> +++ b/src/intel/compiler/brw_nir.h
> @@ -114,6 +114,8 @@ void brw_nir_lower_tcs_outputs(nir_shader *nir, const
> struct brw_vue_map *vue,
> GLenum tes_primitive_mode);
> void brw_nir_lower_fs_outputs(nir_shader *nir);
>
> +bool brw_nir_lower_conversions(nir_shader *nir);
> +
> bool brw_nir_lower_image_load_store(nir_shader *nir,
> const struct gen_device_info
> *devinfo);
> void brw_nir_rewrite_image_intrinsic(nir_intrinsic_instr *intrin,
> diff --git a/src/intel/compiler/brw_nir_lower_conversions.c
> b/src/intel/compiler/brw_nir_lower_conversions.c
> new file mode 100644
> index 000..583167c7753
> --- /dev/null
> +++ b/src/intel/compiler/brw_nir_lower_conversions.c
> @@ -0,0 +1,158 @@
> +/*
> + * Copyright © 2018 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the
> "Software"),
> + * to deal in the Software without restriction, including without
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the
> next
> + * paragraph) shall be included in all copies or substantial portions of
> the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> DEALINGS
> + * IN THE SOFTWARE.
> + */
> +
> +#include "brw_nir.h"
> +#include "compiler/nir/nir_builder.h"
> +
> +static nir_op
> +get_conversion_op(nir_alu_type src_type,
> + unsigned src_bit_size,
> + nir_alu_type dst_type,
> + unsigned dst_bit_size,
> + nir_rounding_mode rounding_mode)
> +{
> + nir_alu_type src_full_type = (nir_alu_type) (src_type | src_bit_size);
> + nir_alu_type dst_full_type = (nir_alu_type) (dst_type | dst_bit_size);
> +
> + return nir_type_conversion_op(src_full_type, dst_full_type,
> rounding_mode);
> +}
> +
> +static nir_rounding_mode
> +get_opcode_rounding_mode(nir_op op)
> +{
> + switch (op) {
> + case nir_op_f2f16_rtz:
> + return
[Mesa-dev] [PATCH v3 02/42] intel/compiler: add a NIR pass to lower conversions
Some conversions are not directly supported in hardware and need to be
split in two conversion instructions going through an intermediary type.
Doing this at the NIR level simplifies a bit the complexity in the backend.
v2:
- Consider fp16 rounding conversion opcodes
- Properly handle swizzles on conversion sources.
Reviewed-by: Topi Pohjolainen (v1)
---
src/intel/Makefile.sources| 1 +
src/intel/compiler/brw_nir.c | 1 +
src/intel/compiler/brw_nir.h | 2 +
.../compiler/brw_nir_lower_conversions.c | 158 ++
src/intel/compiler/meson.build| 1 +
5 files changed, 163 insertions(+)
create mode 100644 src/intel/compiler/brw_nir_lower_conversions.c
diff --git a/src/intel/Makefile.sources b/src/intel/Makefile.sources
index 94a28d370e8..9975daa3ad1 100644
--- a/src/intel/Makefile.sources
+++ b/src/intel/Makefile.sources
@@ -83,6 +83,7 @@ COMPILER_FILES = \
compiler/brw_nir_analyze_boolean_resolves.c \
compiler/brw_nir_analyze_ubo_ranges.c \
compiler/brw_nir_attribute_workarounds.c \
+ compiler/brw_nir_lower_conversions.c \
compiler/brw_nir_lower_cs_intrinsics.c \
compiler/brw_nir_lower_image_load_store.c \
compiler/brw_nir_lower_mem_access_bit_sizes.c \
diff --git a/src/intel/compiler/brw_nir.c b/src/intel/compiler/brw_nir.c
index 92d7fe4bede..572ab824a94 100644
--- a/src/intel/compiler/brw_nir.c
+++ b/src/intel/compiler/brw_nir.c
@@ -882,6 +882,7 @@ brw_postprocess_nir(nir_shader *nir, const struct
brw_compiler *compiler,
OPT(nir_opt_move_comparisons);
OPT(nir_lower_bool_to_int32);
+ OPT(brw_nir_lower_conversions);
OPT(nir_lower_locals_to_regs);
diff --git a/src/intel/compiler/brw_nir.h b/src/intel/compiler/brw_nir.h
index bc81950d47e..662b2627e95 100644
--- a/src/intel/compiler/brw_nir.h
+++ b/src/intel/compiler/brw_nir.h
@@ -114,6 +114,8 @@ void brw_nir_lower_tcs_outputs(nir_shader *nir, const
struct brw_vue_map *vue,
GLenum tes_primitive_mode);
void brw_nir_lower_fs_outputs(nir_shader *nir);
+bool brw_nir_lower_conversions(nir_shader *nir);
+
bool brw_nir_lower_image_load_store(nir_shader *nir,
const struct gen_device_info *devinfo);
void brw_nir_rewrite_image_intrinsic(nir_intrinsic_instr *intrin,
diff --git a/src/intel/compiler/brw_nir_lower_conversions.c
b/src/intel/compiler/brw_nir_lower_conversions.c
new file mode 100644
index 000..583167c7753
--- /dev/null
+++ b/src/intel/compiler/brw_nir_lower_conversions.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright © 2018 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "brw_nir.h"
+#include "compiler/nir/nir_builder.h"
+
+static nir_op
+get_conversion_op(nir_alu_type src_type,
+ unsigned src_bit_size,
+ nir_alu_type dst_type,
+ unsigned dst_bit_size,
+ nir_rounding_mode rounding_mode)
+{
+ nir_alu_type src_full_type = (nir_alu_type) (src_type | src_bit_size);
+ nir_alu_type dst_full_type = (nir_alu_type) (dst_type | dst_bit_size);
+
+ return nir_type_conversion_op(src_full_type, dst_full_type, rounding_mode);
+}
+
+static nir_rounding_mode
+get_opcode_rounding_mode(nir_op op)
+{
+ switch (op) {
+ case nir_op_f2f16_rtz:
+ return nir_rounding_mode_rtz;
+ case nir_op_f2f16_rtne:
+ return nir_rounding_mode_rtne;
+ default:
+ return nir_rounding_mode_undef;
+ }
+}
+
+static void
+split_conversion(nir_builder *b, nir_alu_instr *alu, nir_op op1, nir_op op2)
+{
+ b->cursor = nir_before_instr(>instr);
+ assert(alu->dest.write_mask == 1);
+ nir_ssa_def *src = nir_ssa_for_alu_src(b, alu, 0);
+ nir_ssa_def *tmp = nir_build_alu(b, op1, src, NULL, NULL, NULL);
+ nir_ssa_def *res = nir_build_alu(b, op2, tmp, NULL, NULL, NULL);
