Looks good to me. Please add all necessary test cases to cover all these code paths.
Good stuff guys. Very subtle code indeed. Jose ----- Original Message ----- > From: Roland Scheidegger <srol...@vmware.com> > > If we're in some conditional or loop we must not return, or the code > after the condition is never executed. > (v2): And, we also can't just continue as nothing happened, since the > mask update code would later check if we actually have a mask, so we > need to remember that there was a return in main where we didn't exit > (to illustrate this, a ret in a if clause would cause a mask update > which is still ok as we're in a conditional, but after the endif the > mask update code would drop the mask hence bringing execution back to > pixels which should have their execution mask set to zero by the ret). > Thanks to Christoph Bumiller for figuring this out. > > This fixes https://bugs.freedesktop.org/show_bug.cgi?id=62357. > > Note: This is a candidate for the stable branches. > --- > src/gallium/auxiliary/gallivm/lp_bld_tgsi.h | 1 + > src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c | 20 +++++++++++++++++--- > 2 files changed, 18 insertions(+), 3 deletions(-) > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h > index dac97c3..6e65e12 100644 > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi.h > @@ -243,6 +243,7 @@ struct lp_exec_mask { > struct lp_build_context *bld; > > boolean has_mask; > + boolean ret_in_main; > > LLVMTypeRef int_vec_type; > > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > index 0dc26b5..965255a 100644 > --- a/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > +++ b/src/gallium/auxiliary/gallivm/lp_bld_tgsi_soa.c > @@ -73,6 +73,7 @@ static void lp_exec_mask_init(struct lp_exec_mask *mask, > struct lp_build_context > > mask->bld = bld; > mask->has_mask = FALSE; > + mask->ret_in_main = FALSE; > mask->cond_stack_size = 0; > mask->loop_stack_size = 0; > mask->call_stack_size = 0; > @@ -108,7 +109,7 @@ static void lp_exec_mask_update(struct lp_exec_mask > *mask) > } else > mask->exec_mask = mask->cond_mask; > > - if (mask->call_stack_size) { > + if (mask->call_stack_size || mask->ret_in_main) { > mask->exec_mask = LLVMBuildAnd(builder, > mask->exec_mask, > mask->ret_mask, > @@ -117,7 +118,8 @@ static void lp_exec_mask_update(struct lp_exec_mask > *mask) > > mask->has_mask = (mask->cond_stack_size > 0 || > mask->loop_stack_size > 0 || > - mask->call_stack_size > 0); > + mask->call_stack_size > 0 || > + mask->ret_in_main); > } > > static void lp_exec_mask_cond_push(struct lp_exec_mask *mask, > @@ -348,11 +350,23 @@ static void lp_exec_mask_ret(struct lp_exec_mask *mask, > int *pc) > LLVMBuilderRef builder = mask->bld->gallivm->builder; > LLVMValueRef exec_mask; > > - if (mask->call_stack_size == 0) { > + if (mask->cond_stack_size == 0 && > + mask->loop_stack_size == 0 && > + mask->call_stack_size == 0) { > /* returning from main() */ > *pc = -1; > return; > } > + > + if (mask->call_stack_size == 0) { > + /* > + * This requires special handling since we need to ensure > + * we don't drop the mask even if we have no call stack > + * (e.g. after a ret in a if clause after the endif) > + */ > + mask->ret_in_main = TRUE; > + } > + > exec_mask = LLVMBuildNot(builder, > mask->exec_mask, > "ret"); > -- > 1.7.9.5 > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev