Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
Just saw your patch. I'll review that one then :-)
On Tue, Feb 12, 2019 at 09:38:32AM -0800, Caio Marcelo de Oliveira Filho wrote:
> Hi Juan,
>
> On Tue, Feb 12, 2019 at 04:37:23PM +0100, Juan A. Suarez Romero wrote:
> > On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> > > I had a chat with Caio about this and I'm skeptical. In general, users
> > > of the CF manipulation code shouldn't be stitching two blocks together
> > > where the first contains a jump and the second is non-empty. If the
> > > caller knows that this case is ok, then they can check for it and empty
> > > out the one block before stitching. Also, I'm not really seeing how
> > > peel_initial_if would hit this case from your example.
> > >
> > >
> > The problem happens when moving the continous list to the end of continue
> > block in loop; the former ends in a jump ("break") and the later also ends
> > in a jump ("continue"), so stitch block complains because there will be an
> > instruction (the "continue") after the jump (the "break").
>
> I was investigating this yesterday and attempted to write a MR, could
> you take a look?
>
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/238
>
>
> Caio
Caio
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
On Tue, 2019-02-12 at 09:38 -0800, Caio Marcelo de Oliveira Filho wrote:
> Hi Juan,
>
> On Tue, Feb 12, 2019 at 04:37:23PM +0100, Juan A. Suarez Romero wrote:
> > On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> > > I had a chat with Caio about this and I'm skeptical. In general, users
> > > of the CF manipulation code shouldn't be stitching two blocks together
> > > where the first contains a jump and the second is non-empty. If the
> > > caller knows that this case is ok, then they can check for it and empty
> > > out the one block before stitching. Also, I'm not really seeing how
> > > peel_initial_if would hit this case from your example.
> > >
> > >
> > The problem happens when moving the continous list to the end of continue
> > block in loop; the former ends in a jump ("break") and the later also ends
> > in a jump ("continue"), so stitch block complains because there will be an
> > instruction (the "continue") after the jump (the "break").
>
> I was investigating this yesterday and attempted to write a MR, could
> you take a look?
>
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/238
>
>
I had sent a patch to fix it (https://patchwork.freedesktop.org/patch/285649/)
which is similar to your MR.
Other than that, your MR also fixes the issue.
J.A.
> Caio
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
Hi Juan,
On Tue, Feb 12, 2019 at 04:37:23PM +0100, Juan A. Suarez Romero wrote:
> On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> > I had a chat with Caio about this and I'm skeptical. In general, users of
> > the CF manipulation code shouldn't be stitching two blocks together where
> > the first contains a jump and the second is non-empty. If the caller knows
> > that this case is ok, then they can check for it and empty out the one
> > block before stitching. Also, I'm not really seeing how peel_initial_if
> > would hit this case from your example.
> >
> >
> The problem happens when moving the continous list to the end of continue
> block in loop; the former ends in a jump ("break") and the later also ends in
> a jump ("continue"), so stitch block complains because there will be an
> instruction (the "continue") after the jump (the "break").
I was investigating this yesterday and attempted to write a MR, could
you take a look?
https://gitlab.freedesktop.org/mesa/mesa/merge_requests/238
Caio
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> I had a chat with Caio about this and I'm skeptical. In general, users of
> the CF manipulation code shouldn't be stitching two blocks together where the
> first contains a jump and the second is non-empty. If the caller knows that
> this case is ok, then they can check for it and empty out the one block
> before stitching. Also, I'm not really seeing how peel_initial_if would hit
> this case from your example.
>
>
The problem happens when moving the continous list to the end of continue block
in loop; the former ends in a jump ("break") and the later also ends in a jump
("continue"), so stitch block complains because there will be an instruction
(the "continue") after the jump (the "break").
As you mentioned, maybe the caller can detect this situation and just get rid
of the jump instruction in the continue block, before the stitching. After all,
after the merge it won't never be called.
I'm sending a new patch for this.
J.A.
> --Jason
>
>
> On Fri, Jan 25, 2019 at 11:37 AM Juan A. Suarez Romero
> wrote:
> > When stitching two blocks A and B, where A's last instruction is a jump,
> >
> > it is not required that B is empty; it can be plainly removed.
> >
> >
> >
> > This can happen in a situation like this:
> >
> >
> >
> > vec1 1 ssa_1 = load_const (true)
> >
> > vec1 1 ssa_2 = load_const (false)
> >
> > block block_1:
> >
> > [...]
> >
> > loop {
> >
> > vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> >
> > if ssa_3 {
> >
> > block block_2:
> >
> > [...]
> >
> > break
> >
> > } else {
> >
> > block block_3:
> >
> > }
> >
> > vec1 ssa_4 =
> >
> > if ssa_4 {
> >
> > block block_4:
> >
> > continue
> >
> > } else {
> >
> > block block_5:
> >
> > }
> >
> > block block_6:
> >
> > [...]
> >
> > }
> >
> >
> >
> > And opt_peel_loop_initial_if is applied. In this case, we would be
> >
> > ending up stitching block_2 (which finalizes with a jump) with
> >
> > block_4, which is not empty.
> >
> >
> >
> > CC: Jason Ekstrand
> >
> > ---
> >
> > src/compiler/nir/nir_control_flow.c | 1 -
> >
> > 1 file changed, 1 deletion(-)
> >
> >
> >
> > diff --git a/src/compiler/nir/nir_control_flow.c
> > b/src/compiler/nir/nir_control_flow.c
> >
> > index ddba2e55b45..27508f230d6 100644
> >
> > --- a/src/compiler/nir/nir_control_flow.c
> >
> > +++ b/src/compiler/nir/nir_control_flow.c
> >
> > @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> >
> > */
> >
> >
> >
> > if (nir_block_ends_in_jump(before)) {
> >
> > - assert(exec_list_is_empty(>instr_list));
> >
> >if (after->successors[0])
> >
> > remove_phi_src(after->successors[0], after);
> >
> >if (after->successors[1])
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
On Fri, 2019-02-08 at 15:39 -0600, Jason Ekstrand wrote:
> I had a chat with Caio about this and I'm skeptical. In general, users of
> the CF manipulation code shouldn't be stitching two blocks together where the
> first contains a jump and the second is non-empty. If the caller knows that
> this case is ok, then they can check for it and empty out the one block
> before stitching. Also, I'm not really seeing how peel_initial_if would hit
> this case from your example.
>
I'll re-check again, as with the last changes in Mesa the problem moved to a
different place.
J.A.
> --Jason
>
>
> On Fri, Jan 25, 2019 at 11:37 AM Juan A. Suarez Romero
> wrote:
> > When stitching two blocks A and B, where A's last instruction is a jump,
> >
> > it is not required that B is empty; it can be plainly removed.
> >
> >
> >
> > This can happen in a situation like this:
> >
> >
> >
> > vec1 1 ssa_1 = load_const (true)
> >
> > vec1 1 ssa_2 = load_const (false)
> >
> > block block_1:
> >
> > [...]
> >
> > loop {
> >
> > vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> >
> > if ssa_3 {
> >
> > block block_2:
> >
> > [...]
> >
> > break
> >
> > } else {
> >
> > block block_3:
> >
> > }
> >
> > vec1 ssa_4 =
> >
> > if ssa_4 {
> >
> > block block_4:
> >
> > continue
> >
> > } else {
> >
> > block block_5:
> >
> > }
> >
> > block block_6:
> >
> > [...]
> >
> > }
> >
> >
> >
> > And opt_peel_loop_initial_if is applied. In this case, we would be
> >
> > ending up stitching block_2 (which finalizes with a jump) with
> >
> > block_4, which is not empty.
> >
> >
> >
> > CC: Jason Ekstrand
> >
> > ---
> >
> > src/compiler/nir/nir_control_flow.c | 1 -
> >
> > 1 file changed, 1 deletion(-)
> >
> >
> >
> > diff --git a/src/compiler/nir/nir_control_flow.c
> > b/src/compiler/nir/nir_control_flow.c
> >
> > index ddba2e55b45..27508f230d6 100644
> >
> > --- a/src/compiler/nir/nir_control_flow.c
> >
> > +++ b/src/compiler/nir/nir_control_flow.c
> >
> > @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> >
> > */
> >
> >
> >
> > if (nir_block_ends_in_jump(before)) {
> >
> > - assert(exec_list_is_empty(>instr_list));
> >
> >if (after->successors[0])
> >
> > remove_phi_src(after->successors[0], after);
> >
> >if (after->successors[1])
> >
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
On Fri, 2019-02-08 at 10:29 -0800, Ian Romanick wrote: > On 2/8/19 5:21 AM, Juan A. Suarez Romero wrote: > > On Sat, 2019-01-26 at 08:37 -0800, Jason Ekstrand wrote: > > > This makes me a bit nervous. I'll have to look at it in more detail. > > > > > > > Did you have time to take a look at this? > > Is there a test case that hits this? Was it found by inspection? I'm > curious what the back story is... > Found when dealing with this SPIR-V code: OpCapability Shader %1 = OpExtInstImport "GLSL.std.450" OpMemoryModel Logical GLSL450 OpEntryPoint Fragment %main "main" %_GLF_color %gl_FragCoord OpExecutionMode %main OriginUpperLeft OpSource ESSL 310 OpName %main "main" OpName %_GLF_color "_GLF_color" OpName %gl_FragCoord "gl_FragCoord" OpDecorate %_GLF_color Location 0 OpDecorate %gl_FragCoord BuiltIn FragCoord %void = OpTypeVoid %3 = OpTypeFunction %void %float = OpTypeFloat 32 %v4float = OpTypeVector %float 4 %_ptr_Output_v4float = OpTypePointer Output %v4float %_GLF_color = OpVariable %_ptr_Output_v4float Output %float_1 = OpConstant %float 1 %float_0 = OpConstant %float 0 %12 = OpConstantComposite %v4float %float_1 %float_0 %float_0 %float_1 %int = OpTypeInt 32 1 %int_0 = OpConstant %int 0 %int_1 = OpConstant %int 1 %bool = OpTypeBool %_ptr_Input_v4float = OpTypePointer Input %v4float %gl_FragCoord = OpVariable %_ptr_Input_v4float Input %uint = OpTypeInt 32 0 %uint_0 = OpConstant %uint 0 %_ptr_Input_float = OpTypePointer Input %float %float_40 = OpConstant %float 40 %float_140 = OpConstant %float 140 %float_160 = OpConstant %float 160 %uint_1 = OpConstant %uint 1 %float_180 = OpConstant %float 180 %false = OpConstantFalse %bool %true = OpConstantTrue %bool %181 = OpUndef %v4float %main = OpFunction %void None %3 %5 = OpLabel OpBranch %157 %157 = OpLabel OpStore %_GLF_color %12 OpLoopMerge %156 %159 None OpBranch %17 %17 = OpLabel %167 = OpPhi %int %int_0 %157 %44 %41 %25 = OpSLessThan %bool %167 %int_1 OpLoopMerge %19 %41 None OpBranchConditional %25 %18 %19 %18 = OpLabel %31 = OpAccessChain %_ptr_Input_float %gl_FragCoord %uint_0 %32 = OpLoad %float %31 %33 = OpFOrdLessThan %bool %float_1 %32 OpSelectionMerge %35 None OpBranchConditional %33 %34 %35 %34 = OpLabel OpBranch %19 %35 = OpLabel %39 = OpFOrdLessThan %bool %32 %float_0 OpSelectionMerge %41 None OpBranchConditional %39 %40 %41 %40 = OpLabel OpBranch %19 %41 = OpLabel %44 = OpIAdd %int %167 %int_1 OpBranch %17 %19 = OpLabel %168 = OpPhi %bool %false %17 %false %34 %true %40 OpSelectionMerge %163 None OpBranchConditional %168 %156 %163 %163 = OpLabel OpBranch %45 %45 = OpLabel OpLoopMerge %47 %53 None OpBranch %46 %46 = OpLabel %49 = OpAccessChain %_ptr_Input_float %gl_FragCoord %uint_0 %50 = OpLoad %float %49 %52 = OpFOrdLessThan %bool %50 %float_40 OpSelectionMerge %53 None OpBranchConditional %52 %53 %55 %53 = OpLabel OpStore %_GLF_color %12 OpBranchConditional %false %45 %47 %55 = OpLabel %59 = OpFOrdLessThan %bool %50 %float_140 OpSelectionMerge %61 None OpBranchConditional %59 %60 %62 %60 = OpLabel OpBranch %61 %62 = OpLabel OpBranch %64 %64 = OpLabel %176 = OpPhi %v4float %181 %62 %197 %76 %171 = OpPhi %int %int_1 %62 %153 %76 %70 = OpSGreaterThan %bool %171 %int_0 OpLoopMerge %66 %76 None OpBranchConditional %70 %65 %66 %65 = OpLabel %74 = OpFOrdLessThan %bool %50 %float_160 OpSelectionMerge %76 None OpBranchConditional %74 %75 %94 %75 = OpLabel OpBranch %78 %78 = OpLabel %185 = OpPhi %int %int_1 %75 %93 %90 %84 = OpSGreaterThan %bool %185 %int_0 OpLoopMerge %80 %90 None OpBranchConditional %84 %79 %80 %79 = OpLabel %86 = OpAccessChain %_ptr_Input_float %gl_FragCoord %uint_1 %87 = OpLoad %float %86 %88 = OpFOrdLessThan %bool %87 %float_0 OpSelectionMerge %90 None OpBranchConditional %88 %89 %91 %89 = OpLabel OpBranch %90 %91 = OpLabel
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
I had a chat with Caio about this and I'm skeptical. In general, users of
the CF manipulation code shouldn't be stitching two blocks together where
the first contains a jump and the second is non-empty. If the caller knows
that this case is ok, then they can check for it and empty out the one
block before stitching. Also, I'm not really seeing how peel_initial_if
would hit this case from your example.
--Jason
On Fri, Jan 25, 2019 at 11:37 AM Juan A. Suarez Romero
wrote:
> When stitching two blocks A and B, where A's last instruction is a jump,
> it is not required that B is empty; it can be plainly removed.
>
> This can happen in a situation like this:
>
> vec1 1 ssa_1 = load_const (true)
> vec1 1 ssa_2 = load_const (false)
> block block_1:
> [...]
> loop {
> vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> if ssa_3 {
> block block_2:
> [...]
> break
> } else {
> block block_3:
> }
> vec1 ssa_4 =
> if ssa_4 {
> block block_4:
> continue
> } else {
> block block_5:
> }
> block block_6:
> [...]
> }
>
> And opt_peel_loop_initial_if is applied. In this case, we would be
> ending up stitching block_2 (which finalizes with a jump) with
> block_4, which is not empty.
>
> CC: Jason Ekstrand
> ---
> src/compiler/nir/nir_control_flow.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_control_flow.c
> b/src/compiler/nir/nir_control_flow.c
> index ddba2e55b45..27508f230d6 100644
> --- a/src/compiler/nir/nir_control_flow.c
> +++ b/src/compiler/nir/nir_control_flow.c
> @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> */
>
> if (nir_block_ends_in_jump(before)) {
> - assert(exec_list_is_empty(>instr_list));
>if (after->successors[0])
> remove_phi_src(after->successors[0], after);
>if (after->successors[1])
> --
> 2.20.1
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
On 2/8/19 5:21 AM, Juan A. Suarez Romero wrote:
> On Sat, 2019-01-26 at 08:37 -0800, Jason Ekstrand wrote:
>> This makes me a bit nervous. I'll have to look at it in more detail.
>>
>
> Did you have time to take a look at this?
Is there a test case that hits this? Was it found by inspection? I'm
curious what the back story is...
> J.A.
>
>> On January 25, 2019 09:37:52 "Juan A. Suarez Romero"
>> wrote:
>>
>>> When stitching two blocks A and B, where A's last instruction is a jump,
>>> it is not required that B is empty; it can be plainly removed.
>>>
>>> This can happen in a situation like this:
>>>
>>> vec1 1 ssa_1 = load_const (true)
>>> vec1 1 ssa_2 = load_const (false)
>>> block block_1:
>>> [...]
>>> loop {
>>> vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
>>> if ssa_3 {
>>>block block_2:
>>>[...]
>>>break
>>> } else {
>>>block block_3:
>>> }
>>> vec1 ssa_4 =
>>> if ssa_4 {
>>>block block_4:
>>>continue
>>> } else {
>>>block block_5:
>>> }
>>> block block_6:
>>> [...]
>>> }
>>>
>>> And opt_peel_loop_initial_if is applied. In this case, we would be
>>> ending up stitching block_2 (which finalizes with a jump) with
>>> block_4, which is not empty.
>>>
>>> CC: Jason Ekstrand
>>> ---
>>> src/compiler/nir/nir_control_flow.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/src/compiler/nir/nir_control_flow.c
>>> b/src/compiler/nir/nir_control_flow.c
>>> index ddba2e55b45..27508f230d6 100644
>>> --- a/src/compiler/nir/nir_control_flow.c
>>> +++ b/src/compiler/nir/nir_control_flow.c
>>> @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
>>> */
>>>
>>>if (nir_block_ends_in_jump(before)) {
>>> - assert(exec_list_is_empty(>instr_list));
>>> if (after->successors[0])
>>> remove_phi_src(after->successors[0], after);
>>> if (after->successors[1])
>>> --
>>> 2.20.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: allow stitching of non-empty block
On Sat, 2019-01-26 at 08:37 -0800, Jason Ekstrand wrote:
> This makes me a bit nervous. I'll have to look at it in more detail.
>
Did you have time to take a look at this?
J.A.
> On January 25, 2019 09:37:52 "Juan A. Suarez Romero"
> wrote:
>
> > When stitching two blocks A and B, where A's last instruction is a jump,
> > it is not required that B is empty; it can be plainly removed.
> >
> > This can happen in a situation like this:
> >
> > vec1 1 ssa_1 = load_const (true)
> > vec1 1 ssa_2 = load_const (false)
> > block block_1:
> > [...]
> > loop {
> > vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> > if ssa_3 {
> >block block_2:
> >[...]
> >break
> > } else {
> >block block_3:
> > }
> > vec1 ssa_4 =
> > if ssa_4 {
> >block block_4:
> >continue
> > } else {
> >block block_5:
> > }
> > block block_6:
> > [...]
> > }
> >
> > And opt_peel_loop_initial_if is applied. In this case, we would be
> > ending up stitching block_2 (which finalizes with a jump) with
> > block_4, which is not empty.
> >
> > CC: Jason Ekstrand
> > ---
> > src/compiler/nir/nir_control_flow.c | 1 -
> > 1 file changed, 1 deletion(-)
> >
> > diff --git a/src/compiler/nir/nir_control_flow.c
> > b/src/compiler/nir/nir_control_flow.c
> > index ddba2e55b45..27508f230d6 100644
> > --- a/src/compiler/nir/nir_control_flow.c
> > +++ b/src/compiler/nir/nir_control_flow.c
> > @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> > */
> >
> >if (nir_block_ends_in_jump(before)) {
> > - assert(exec_list_is_empty(>instr_list));
> > if (after->successors[0])
> > remove_phi_src(after->successors[0], after);
> > if (after->successors[1])
> > --
> > 2.20.1
>
>
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
This makes me a bit nervous. I'll have to look at it in more detail.
On January 25, 2019 09:37:52 "Juan A. Suarez Romero"
wrote:
When stitching two blocks A and B, where A's last instruction is a jump,
it is not required that B is empty; it can be plainly removed.
This can happen in a situation like this:
vec1 1 ssa_1 = load_const (true)
vec1 1 ssa_2 = load_const (false)
block block_1:
[...]
loop {
vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
if ssa_3 {
block block_2:
[...]
break
} else {
block block_3:
}
vec1 ssa_4 =
if ssa_4 {
block block_4:
continue
} else {
block block_5:
}
block block_6:
[...]
}
And opt_peel_loop_initial_if is applied. In this case, we would be
ending up stitching block_2 (which finalizes with a jump) with
block_4, which is not empty.
CC: Jason Ekstrand
---
src/compiler/nir/nir_control_flow.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/compiler/nir/nir_control_flow.c
b/src/compiler/nir/nir_control_flow.c
index ddba2e55b45..27508f230d6 100644
--- a/src/compiler/nir/nir_control_flow.c
+++ b/src/compiler/nir/nir_control_flow.c
@@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
*/
if (nir_block_ends_in_jump(before)) {
- assert(exec_list_is_empty(>instr_list));
if (after->successors[0])
remove_phi_src(after->successors[0], after);
if (after->successors[1])
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] nir: allow stitching of non-empty block
This patch is
Reviewed-by: Caio Marcelo de Oliveira Filho
On Fri, Jan 25, 2019 at 06:37:33PM +0100, Juan A. Suarez Romero wrote:
> When stitching two blocks A and B, where A's last instruction is a jump,
> it is not required that B is empty; it can be plainly removed.
>
> This can happen in a situation like this:
>
> vec1 1 ssa_1 = load_const (true)
> vec1 1 ssa_2 = load_const (false)
> block block_1:
> [...]
> loop {
> vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
> if ssa_3 {
> block block_2:
> [...]
> break
> } else {
> block block_3:
> }
> vec1 ssa_4 =
> if ssa_4 {
> block block_4:
> continue
> } else {
> block block_5:
> }
> block block_6:
> [...]
> }
>
> And opt_peel_loop_initial_if is applied. In this case, we would be
> ending up stitching block_2 (which finalizes with a jump) with
> block_4, which is not empty.
>
> CC: Jason Ekstrand
> ---
> src/compiler/nir/nir_control_flow.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/src/compiler/nir/nir_control_flow.c
> b/src/compiler/nir/nir_control_flow.c
> index ddba2e55b45..27508f230d6 100644
> --- a/src/compiler/nir/nir_control_flow.c
> +++ b/src/compiler/nir/nir_control_flow.c
> @@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
> */
>
> if (nir_block_ends_in_jump(before)) {
> - assert(exec_list_is_empty(>instr_list));
>if (after->successors[0])
> remove_phi_src(after->successors[0], after);
>if (after->successors[1])
> --
> 2.20.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Caio
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] nir: allow stitching of non-empty block
When stitching two blocks A and B, where A's last instruction is a jump,
it is not required that B is empty; it can be plainly removed.
This can happen in a situation like this:
vec1 1 ssa_1 = load_const (true)
vec1 1 ssa_2 = load_const (false)
block block_1:
[...]
loop {
vec1 ssa_3 = phi block_1: ssa_2, block_4: ssa_1
if ssa_3 {
block block_2:
[...]
break
} else {
block block_3:
}
vec1 ssa_4 =
if ssa_4 {
block block_4:
continue
} else {
block block_5:
}
block block_6:
[...]
}
And opt_peel_loop_initial_if is applied. In this case, we would be
ending up stitching block_2 (which finalizes with a jump) with
block_4, which is not empty.
CC: Jason Ekstrand
---
src/compiler/nir/nir_control_flow.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/src/compiler/nir/nir_control_flow.c
b/src/compiler/nir/nir_control_flow.c
index ddba2e55b45..27508f230d6 100644
--- a/src/compiler/nir/nir_control_flow.c
+++ b/src/compiler/nir/nir_control_flow.c
@@ -550,7 +550,6 @@ stitch_blocks(nir_block *before, nir_block *after)
*/
if (nir_block_ends_in_jump(before)) {
- assert(exec_list_is_empty(>instr_list));
if (after->successors[0])
remove_phi_src(after->successors[0], after);
if (after->successors[1])
--
2.20.1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
