<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Dec 19, 2016 at 8:18 PM, Timothy Arceri <span dir="ltr"><<a href="mailto:timothy.arceri@collabora.com" target="_blank">timothy.arceri@collabora.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Unless an if statement contains nested returns we can simply add<br>
any following instructions to the branch without the return.<br>
<br>
V2: fix handling if_nested_return value when there is a sibling if/loop<br>
that doesn't contain a return. (Spotted by Ken)<br>
---<br>
 src/compiler/nir/nir_lower_<wbr>returns.c | 37 ++++++++++++++++++++++++++++++<wbr>------<br>
 1 file changed, 31 insertions(+), 6 deletions(-)<br>
<br>
diff --git a/src/compiler/nir/nir_lower_<wbr>returns.c b/src/compiler/nir/nir_lower_<wbr>returns.c<br>
index cf49d5b..5eec984 100644<br>
--- a/src/compiler/nir/nir_lower_<wbr>returns.c<br>
+++ b/src/compiler/nir/nir_lower_<wbr>returns.c<br>
@@ -30,6 +30,8 @@ struct lower_returns_state {<br>
    struct exec_list *cf_list;<br>
    nir_loop *loop;<br>
    nir_variable *return_flag;<br>
+   /* Are there other return statments nested in the current if */<br>
+   bool if_nested_return;<br>
 };<br>
<br>
 static bool lower_returns_in_cf_list(<wbr>struct exec_list *cf_list,<br>
@@ -82,8 +84,10 @@ lower_returns_in_loop(nir_loop *loop, struct lower_returns_state *state)<br>
     * flag set to true.  We need to predicate everything following the loop<br>
     * on the return flag.<br>
     */<br>
-   if (progress)<br>
+   if (progress) {<br>
       predicate_following(&loop->cf_<wbr>node, state);<br>
+      state->if_nested_return = true;<br>
+   }<br>
<br>
    return progress;<br>
 }<br>
@@ -91,10 +95,13 @@ lower_returns_in_loop(nir_loop *loop, struct lower_returns_state *state)<br>
 static bool<br>
 lower_returns_in_if(nir_if *if_stmt, struct lower_returns_state *state)<br>
 {<br>
-   bool progress;<br>
+   bool progress, then_progress;<br>
<br>
-   progress = lower_returns_in_cf_list(&if_<wbr>stmt->then_list, state);<br>
-   progress = lower_returns_in_cf_list(&if_<wbr>stmt->else_list, state) || progress;<br>
+   bool if_nested_return = state->if_nested_return;<br>
+   state->if_nested_return = false;<br>
+<br>
+   then_progress = lower_returns_in_cf_list(&if_<wbr>stmt->then_list, state);<br>
+   progress = lower_returns_in_cf_list(&if_<wbr>stmt->else_list, state) || then_progress;<br></blockquote><div><br></div><div>I don't really get why we need this if_nested_return thing.  Why can't we just have two progress booleans called then_progress and else_progress and just do<br><br></div><div>if (then_progress && else_progress) {<br></div><div>   predicate_following<br></div><div>} else if (!then_progress && !else_progress) {<br></div><div>   return false;<br></div><div>} else {<br></div><div>   /* Put it in one side or the other based on progress */<br></div><div>}<br><br></div><div>That seems way simpler.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
    /* If either of the recursive calls made progress, then there were<br>
     * returns inside of the body of the if.  If we're in a loop, then these<br>
@@ -106,8 +113,25 @@ lower_returns_in_if(nir_if *if_stmt, struct lower_returns_state *state)<br>
     * after a return, we need to predicate everything following on the<br>
     * return flag.<br>
     */<br>
-   if (progress && !state->loop)<br>
-      predicate_following(&if_stmt-><wbr>cf_node, state);<br>
+   if (progress && !state->loop) {<br>
+      if (state->if_nested_return) {<br>
+         predicate_following(&if_stmt-><wbr>cf_node, state);<br>
+      } else {<br>
+         /* If there are no nested returns we can just add the instructions to<br>
+          * the end of the branch that doesn't have the return.<br>
+          */<br>
+         nir_cf_list list;<br>
+         nir_cf_extract(&list, nir_after_cf_node(&if_stmt-><wbr>cf_node),<br>
+                        nir_after_cf_list(state->cf_<wbr>list));<br>
+<br>
+         if (then_progress)<br>
+            nir_cf_reinsert(&list, nir_after_cf_list(&if_stmt-><wbr>else_list));<br>
+         else<br>
+            nir_cf_reinsert(&list, nir_after_cf_list(&if_stmt-><wbr>then_list));<br>
+      }<br>
+   }<br>
+<br>
+   state->if_nested_return = progress || if_nested_return;<br>
<br>
    return progress;<br>
 }<br>
@@ -221,6 +245,7 @@ nir_lower_returns_impl(nir_<wbr>function_impl *impl)<br>
    state.cf_list = &impl->body;<br>
    state.loop = NULL;<br>
    state.return_flag = NULL;<br>
+   state.if_nested_return = false;<br>
    nir_builder_init(&state.<wbr>builder, impl);<br>
<br>
    bool progress = lower_returns_in_cf_list(&<wbr>impl->body, &state);<br>
<span class="HOEnZb"><font color="#888888">--<br>
2.9.3<br>
<br>
______________________________<wbr>_________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>