Re: [Mesa-dev] [PATCH 2/4] anv/apply_dynamic_offsets: Use rewrite_src instead of a regular assignment
Hi Jason,
Am 19.05.2016 um 06:42 schrieb Jason Ekstrand:
> Originally we removed the instruction, changed the source, and then
> re-inserted it. This works, but nir_instr_rewrite_src is a bit more
> obviously correct.
> ---
> src/intel/vulkan/anv_nir_apply_dynamic_offsets.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> index 6a28953..a7ed3e5 100644
> --- a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> +++ b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> @@ -80,9 +80,10 @@ apply_dynamic_offsets_block(nir_block *block, nir_builder
> *b,
>nir_ssa_dest_init(&offset_load->instr, &offset_load->dest, 2, 32,
> NULL);
>nir_builder_instr_insert(b, &offset_load->instr);
>
> - nir_src *offset_src = nir_get_io_offset_src(intrin);
I think you didnt want to remove *offset_src here, given that it is used
below and reintroduced in the next patch?
--Michael
> - nir_ssa_def *new_offset = nir_iadd(b, offset_src->ssa,
> + nir_ssa_def *new_offset = nir_iadd(b, nir_ssa_for_src(*offset_src),
> &offset_load->dest.ssa);
> + nir_instr_rewrite_src(&intrin->instr, nir_get_io_offset_src(intrin),
> +nir_src_for_ssa(new_offset));
>
>/* In order to avoid out-of-bounds access, we predicate */
>nir_ssa_def *pred = nir_uge(b, nir_channel(b, &offset_load->dest.ssa,
> 1),
> @@ -92,7 +93,6 @@ apply_dynamic_offsets_block(nir_block *block, nir_builder
> *b,
>nir_cf_node_insert(b->cursor, &if_stmt->cf_node);
>
>nir_instr_remove(&intrin->instr);
> - *offset_src = nir_src_for_ssa(new_offset);
>nir_instr_insert_after_cf_list(&if_stmt->then_list, &intrin->instr);
>
>if (intrin->intrinsic != nir_intrinsic_store_ssbo) {
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/4] anv/apply_dynamic_offsets: Use rewrite_src instead of a regular assignment
On Wed, May 18, 2016 at 11:50 PM, Michael Schellenberger Costa <
mschellenbergerco...@googlemail.com> wrote:
> Hi Jason,
>
> Am 19.05.2016 um 06:42 schrieb Jason Ekstrand:
> > Originally we removed the instruction, changed the source, and then
> > re-inserted it. This works, but nir_instr_rewrite_src is a bit more
> > obviously correct.
> > ---
> > src/intel/vulkan/anv_nir_apply_dynamic_offsets.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> > index 6a28953..a7ed3e5 100644
> > --- a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> > +++ b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
> > @@ -80,9 +80,10 @@ apply_dynamic_offsets_block(nir_block *block,
> nir_builder *b,
> >nir_ssa_dest_init(&offset_load->instr, &offset_load->dest, 2, 32,
> NULL);
> >nir_builder_instr_insert(b, &offset_load->instr);
> >
> > - nir_src *offset_src = nir_get_io_offset_src(intrin);
> I think you didnt want to remove *offset_src here, given that it is used
> below and reintroduced in the next patch?
>
Ugh... Thanks. This patch doesn't even build. I knew that series needed a
little work before I sent it :-)
> --Michael
> > - nir_ssa_def *new_offset = nir_iadd(b, offset_src->ssa,
> > + nir_ssa_def *new_offset = nir_iadd(b,
> nir_ssa_for_src(*offset_src),
> > &offset_load->dest.ssa);
> > + nir_instr_rewrite_src(&intrin->instr,
> nir_get_io_offset_src(intrin),
> > +nir_src_for_ssa(new_offset));
> >
> >/* In order to avoid out-of-bounds access, we predicate */
> >nir_ssa_def *pred = nir_uge(b, nir_channel(b,
> &offset_load->dest.ssa, 1),
> > @@ -92,7 +93,6 @@ apply_dynamic_offsets_block(nir_block *block,
> nir_builder *b,
> >nir_cf_node_insert(b->cursor, &if_stmt->cf_node);
> >
> >nir_instr_remove(&intrin->instr);
> > - *offset_src = nir_src_for_ssa(new_offset);
> >nir_instr_insert_after_cf_list(&if_stmt->then_list,
> &intrin->instr);
> >
> >if (intrin->intrinsic != nir_intrinsic_store_ssbo) {
> >
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/4] anv/apply_dynamic_offsets: Use rewrite_src instead of a regular assignment
Originally we removed the instruction, changed the source, and then
re-inserted it. This works, but nir_instr_rewrite_src is a bit more
obviously correct.
---
src/intel/vulkan/anv_nir_apply_dynamic_offsets.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
index 6a28953..a7ed3e5 100644
--- a/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
+++ b/src/intel/vulkan/anv_nir_apply_dynamic_offsets.c
@@ -80,9 +80,10 @@ apply_dynamic_offsets_block(nir_block *block, nir_builder *b,
nir_ssa_dest_init(&offset_load->instr, &offset_load->dest, 2, 32, NULL);
nir_builder_instr_insert(b, &offset_load->instr);
- nir_src *offset_src = nir_get_io_offset_src(intrin);
- nir_ssa_def *new_offset = nir_iadd(b, offset_src->ssa,
+ nir_ssa_def *new_offset = nir_iadd(b, nir_ssa_for_src(*offset_src),
&offset_load->dest.ssa);
+ nir_instr_rewrite_src(&intrin->instr, nir_get_io_offset_src(intrin),
+nir_src_for_ssa(new_offset));
/* In order to avoid out-of-bounds access, we predicate */
nir_ssa_def *pred = nir_uge(b, nir_channel(b, &offset_load->dest.ssa, 1),
@@ -92,7 +93,6 @@ apply_dynamic_offsets_block(nir_block *block, nir_builder *b,
nir_cf_node_insert(b->cursor, &if_stmt->cf_node);
nir_instr_remove(&intrin->instr);
- *offset_src = nir_src_for_ssa(new_offset);
nir_instr_insert_after_cf_list(&if_stmt->then_list, &intrin->instr);
if (intrin->intrinsic != nir_intrinsic_store_ssbo) {
--
2.5.0.400.gff86faf
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
