Re: [Mesa-dev] [PATCH] nir: fix condition propagation when src has a swizzle
On November 2, 2018 08:20:34 Timothy Arceri wrote: On 2/11/18 11:52 pm, Jason Ekstrand wrote: On November 2, 2018 06:25:59 Timothy Arceri wrote: We cannot use nir_build_alu() to create the new alu as it has no way to know how many components of the src we will use. This results in it guessing the max number of components from one of its inputs. Fixes the following CTS tests: dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Fixes: 2975422ceb6c ("nir: propagates if condition evaluation down some alu chains") --- I'm running this in Intel CI currently but it hasn't been giving me proper results today so fingers crossed. src/compiler/nir/nir_opt_if.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index ed93cac9ce9..1eb49a64aba 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -391,6 +391,34 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value) } } +static nir_ssa_def * +clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu, + nir_ssa_def **src_defs) +{ + nir_alu_instr *nalu = nir_alu_instr_create(b->shader, alu->op); + nalu->exact = alu->exact; + + nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest, + alu->dest.dest.ssa.num_components, + alu->dest.dest.ssa.bit_size, alu->dest.dest.ssa.name); + + nalu->dest.saturate = alu->dest.saturate; + nalu->dest.write_mask = alu->dest.write_mask; Somewhere, we should copy over alu->exact Yeah its at the very top :) Drp... + + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) { + nalu->src[i].src.is_ssa = alu->src[i].src.is_ssa; I think it would be better to assert here + nalu->src[i].src.ssa = src_defs[i]; And use src_for_ssa here Yeah makes sense will change. Cool. Rb then. + nalu->src[i].negate = alu->src[i].negate; + nalu->src[i].abs = alu->src[i].abs; + memcpy(nalu->src[i].swizzle, alu->src[i].swizzle, + sizeof(nalu->src[i].swizzle)); + } + + nir_builder_instr_insert(b, &nalu->instr); + + return &nalu->dest.dest.ssa;; +} + /* * This propagates if condition evaluation down the chain of some alu * instructions. For example by checking the use of some of the following alu @@ -456,7 +484,8 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, def[i] = alu->src[i].src.ssa; } } - nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], def[3]); + + nir_ssa_def *nalu = clone_alu_and_replace_src_defs(b, alu, def); /* Rewrite use to use new alu instruction */ nir_src new_src = nir_src_for_ssa(nalu); -- 2.19.1 ___ 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
Re: [Mesa-dev] [PATCH] nir: fix condition propagation when src has a swizzle
On 2/11/18 11:52 pm, Jason Ekstrand wrote: On November 2, 2018 06:25:59 Timothy Arceri wrote: We cannot use nir_build_alu() to create the new alu as it has no way to know how many components of the src we will use. This results in it guessing the max number of components from one of its inputs. Fixes the following CTS tests: dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Fixes: 2975422ceb6c ("nir: propagates if condition evaluation down some alu chains") --- I'm running this in Intel CI currently but it hasn't been giving me proper results today so fingers crossed. src/compiler/nir/nir_opt_if.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index ed93cac9ce9..1eb49a64aba 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -391,6 +391,34 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value) } } +static nir_ssa_def * +clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu, + nir_ssa_def **src_defs) +{ + nir_alu_instr *nalu = nir_alu_instr_create(b->shader, alu->op); + nalu->exact = alu->exact; + + nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest, + alu->dest.dest.ssa.num_components, + alu->dest.dest.ssa.bit_size, alu->dest.dest.ssa.name); + + nalu->dest.saturate = alu->dest.saturate; + nalu->dest.write_mask = alu->dest.write_mask; Somewhere, we should copy over alu->exact Yeah its at the very top :) + + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) { + nalu->src[i].src.is_ssa = alu->src[i].src.is_ssa; I think it would be better to assert here + nalu->src[i].src.ssa = src_defs[i]; And use src_for_ssa here Yeah makes sense will change. + nalu->src[i].negate = alu->src[i].negate; + nalu->src[i].abs = alu->src[i].abs; + memcpy(nalu->src[i].swizzle, alu->src[i].swizzle, + sizeof(nalu->src[i].swizzle)); + } + + nir_builder_instr_insert(b, &nalu->instr); + + return &nalu->dest.dest.ssa;; +} + /* * This propagates if condition evaluation down the chain of some alu * instructions. For example by checking the use of some of the following alu @@ -456,7 +484,8 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, def[i] = alu->src[i].src.ssa; } } - nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], def[3]); + + nir_ssa_def *nalu = clone_alu_and_replace_src_defs(b, alu, def); /* Rewrite use to use new alu instruction */ nir_src new_src = nir_src_for_ssa(nalu); -- 2.19.1 ___ 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
Re: [Mesa-dev] [PATCH] nir: fix condition propagation when src has a swizzle
On November 2, 2018 06:25:59 Timothy Arceri wrote: We cannot use nir_build_alu() to create the new alu as it has no way to know how many components of the src we will use. This results in it guessing the max number of components from one of its inputs. Fixes the following CTS tests: dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Fixes: 2975422ceb6c ("nir: propagates if condition evaluation down some alu chains") --- I'm running this in Intel CI currently but it hasn't been giving me proper results today so fingers crossed. src/compiler/nir/nir_opt_if.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index ed93cac9ce9..1eb49a64aba 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -391,6 +391,34 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value) } } +static nir_ssa_def * +clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu, + nir_ssa_def **src_defs) +{ + nir_alu_instr *nalu = nir_alu_instr_create(b->shader, alu->op); + nalu->exact = alu->exact; + + nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest, + alu->dest.dest.ssa.num_components, + alu->dest.dest.ssa.bit_size, alu->dest.dest.ssa.name); + + nalu->dest.saturate = alu->dest.saturate; + nalu->dest.write_mask = alu->dest.write_mask; Somewhere, we should copy over alu->exact + + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) { + nalu->src[i].src.is_ssa = alu->src[i].src.is_ssa; I think it would be better to assert here + nalu->src[i].src.ssa = src_defs[i]; And use src_for_ssa here + nalu->src[i].negate = alu->src[i].negate; + nalu->src[i].abs = alu->src[i].abs; + memcpy(nalu->src[i].swizzle, alu->src[i].swizzle, + sizeof(nalu->src[i].swizzle)); + } + + nir_builder_instr_insert(b, &nalu->instr); + + return &nalu->dest.dest.ssa;; +} + /* * This propagates if condition evaluation down the chain of some alu * instructions. For example by checking the use of some of the following alu @@ -456,7 +484,8 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, def[i] = alu->src[i].src.ssa; } } - nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], def[3]); + + nir_ssa_def *nalu = clone_alu_and_replace_src_defs(b, alu, def); /* Rewrite use to use new alu instruction */ nir_src new_src = nir_src_for_ssa(nalu); -- 2.19.1 ___ 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
[Mesa-dev] [PATCH] nir: fix condition propagation when src has a swizzle
We cannot use nir_build_alu() to create the new alu as it has no way to know how many components of the src we will use. This results in it guessing the max number of components from one of its inputs. Fixes the following CTS tests: dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_frag dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_geom dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_tessc dEQP-VK.spirv_assembly.instruction.graphics.selection_block_order.out_of_order_vert Fixes: 2975422ceb6c ("nir: propagates if condition evaluation down some alu chains") --- I'm running this in Intel CI currently but it hasn't been giving me proper results today so fingers crossed. src/compiler/nir/nir_opt_if.c | 31 ++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/src/compiler/nir/nir_opt_if.c b/src/compiler/nir/nir_opt_if.c index ed93cac9ce9..1eb49a64aba 100644 --- a/src/compiler/nir/nir_opt_if.c +++ b/src/compiler/nir/nir_opt_if.c @@ -391,6 +391,34 @@ evaluate_if_condition(nir_if *nif, nir_cursor cursor, bool *value) } } +static nir_ssa_def * +clone_alu_and_replace_src_defs(nir_builder *b, const nir_alu_instr *alu, + nir_ssa_def **src_defs) +{ + nir_alu_instr *nalu = nir_alu_instr_create(b->shader, alu->op); + nalu->exact = alu->exact; + + nir_ssa_dest_init(&nalu->instr, &nalu->dest.dest, + alu->dest.dest.ssa.num_components, + alu->dest.dest.ssa.bit_size, alu->dest.dest.ssa.name); + + nalu->dest.saturate = alu->dest.saturate; + nalu->dest.write_mask = alu->dest.write_mask; + + for (unsigned i = 0; i < nir_op_infos[alu->op].num_inputs; i++) { + nalu->src[i].src.is_ssa = alu->src[i].src.is_ssa; + nalu->src[i].src.ssa = src_defs[i]; + nalu->src[i].negate = alu->src[i].negate; + nalu->src[i].abs = alu->src[i].abs; + memcpy(nalu->src[i].swizzle, alu->src[i].swizzle, + sizeof(nalu->src[i].swizzle)); + } + + nir_builder_instr_insert(b, &nalu->instr); + + return &nalu->dest.dest.ssa;; +} + /* * This propagates if condition evaluation down the chain of some alu * instructions. For example by checking the use of some of the following alu @@ -456,7 +484,8 @@ propagate_condition_eval(nir_builder *b, nir_if *nif, nir_src *use_src, def[i] = alu->src[i].src.ssa; } } - nir_ssa_def *nalu = nir_build_alu(b, alu->op, def[0], def[1], def[2], def[3]); + + nir_ssa_def *nalu = clone_alu_and_replace_src_defs(b, alu, def); /* Rewrite use to use new alu instruction */ nir_src new_src = nir_src_for_ssa(nalu); -- 2.19.1 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev