On Mon, Apr 23, 2018 at 2:20 AM, Timothy Arceri <[email protected]> wrote: > > > On 23/04/18 03:26, Bas Nieuwenhuizen wrote: >> >> We seems to use progress for two cases: > > > seems -> seem > >> 1) When we lowered some returns. >> 2) When we remove unreachable code. >> >> If just case 2 happens we assert as state->return_flag has not >> been allocated yet, but we are still trying to do insert all >> predicates based on it. >> >> This splits the concerns. We only use progress internally for case 1 >> and then keep track of 2 in a separate variable to indicate progress >> in the return value of the pass. >> >> This is slightly better than transforming the assert into >> if (!state->return_flag) return, as it avoids inserting predicates > > > "as it avoids" -> "as that would avoid" ?
No, the solution I am proposing in this patch avoids, not the alternative solution of this paragraph. I think your proposed change would point to the latter. I'll clarify it to clearly point to the former though. > >> even if some other part of the code might need them. >> >> Fixes: 6e22ad6edc "nir: return early when lowering a return at the end of >> a function" >> CC: 18.1 <[email protected]> > > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106174 > > Reviewed-by: Timothy Arceri <[email protected]> > > Thanks for fixing! > > >> --- >> src/compiler/nir/nir_lower_returns.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/src/compiler/nir/nir_lower_returns.c >> b/src/compiler/nir/nir_lower_returns.c >> index 3ea69e2520..9c4881112e 100644 >> --- a/src/compiler/nir/nir_lower_returns.c >> +++ b/src/compiler/nir/nir_lower_returns.c >> @@ -37,6 +37,8 @@ struct lower_returns_state { >> * needs to be predicated on the return flag variable. >> */ >> bool has_predicated_return; >> + >> + bool removed_unreachable_code; >> }; >> static bool lower_returns_in_cf_list(struct exec_list *cf_list, >> @@ -162,8 +164,9 @@ lower_returns_in_block(nir_block *block, struct >> lower_returns_state *state) >> */ >> return false; >> } else { >> + state->removed_unreachable_code = true; >> nir_cf_delete(&list); >> - return true; >> + return false; >> } >> } >> @@ -262,9 +265,11 @@ nir_lower_returns_impl(nir_function_impl *impl) >> state.loop = NULL; >> state.return_flag = NULL; >> state.has_predicated_return = false; >> + state.removed_unreachable_code = false; >> nir_builder_init(&state.builder, impl); >> bool progress = lower_returns_in_cf_list(&impl->body, &state); >> + progress = progress || state.removed_unreachable_code; >> if (progress) { >> nir_metadata_preserve(impl, nir_metadata_none); >> > _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
