Am 29.01.2018 um 13:15 schrieb Gert Wollny: > sb optimized away the ELSE in a construct like > > while (foo) { > if (bar ) { > do something; > } else { > break; > } > } > > resulting in > > while (foo) { > if (bar ) { > do something; > break; > } > } > > which is obviously wrong. > > With this patch an ELSE is inserted also if there was one before optimization. > This fixes piglit shaders@ssa@fs-if-def-else-break. > > It might introduce a penalty by keeping an ELSE instruction when it can > actually be really optimized away though.
Am I correct assuming that for something like while (foo) { if (bar) { do something; } else { /* nothing */ } } The else clause wouldn't get optimized away neither? (This of course is a trivial example, but I suppose it would extend to cases where the optimizer actually optimized away the alu instructions in the else clause). In this case, this indeed looks rather harsh. I would have said in theory somehow the break should be some op which (despite not having a dst) has side-effects so it would not be subject to elimination somewhere (as there would be a if->next node in this case then). Albeit I'm completely oblivious how it really gets optimized away, is that based on liveness analysis or something? The sb code is a mystery to me... Roland > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101442 > --- > I think there should be a better way to ensure that the else is inserted > in this case, but so far it elluded me. > > I have no mesa-git write access. > > src/gallium/drivers/r600/sb/sb_bc_finalize.cpp | 4 +--- > src/gallium/drivers/r600/sb/sb_bc_parser.cpp | 5 ++++- > src/gallium/drivers/r600/sb/sb_ir.h | 3 ++- > 3 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp > b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp > index 099b295f18..4450247caf 100644 > --- a/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp > +++ b/src/gallium/drivers/r600/sb/sb_bc_finalize.cpp > @@ -208,9 +208,7 @@ void bc_finalizer::finalize_if(region_node* r) { > r->push_front(if_jump); > r->push_back(if_pop); > > - bool has_else = n_if->next; > - > - if (has_else) { > + if (n_if->has_else || n_if->next) { > cf_node *nelse = sh.create_cf(CF_OP_ELSE); > n_if->insert_after(nelse); > if_jump->jump(nelse); > diff --git a/src/gallium/drivers/r600/sb/sb_bc_parser.cpp > b/src/gallium/drivers/r600/sb/sb_bc_parser.cpp > index 970e4141d5..f37067b8e2 100644 > --- a/src/gallium/drivers/r600/sb/sb_bc_parser.cpp > +++ b/src/gallium/drivers/r600/sb/sb_bc_parser.cpp > @@ -959,8 +959,11 @@ int bc_parser::prepare_if(cf_node* c) { > > c->insert_before(reg); > > - if (c_else != end) > + if (c_else != end) { > + n_if->has_else = true; > dep->move(c_else, end); > + } > + > dep2->move(c, end); > > reg->push_back(dep); > diff --git a/src/gallium/drivers/r600/sb/sb_ir.h > b/src/gallium/drivers/r600/sb/sb_ir.h > index bee947504e..af9b161597 100644 > --- a/src/gallium/drivers/r600/sb/sb_ir.h > +++ b/src/gallium/drivers/r600/sb/sb_ir.h > @@ -1091,12 +1091,13 @@ public: > > class if_node : public container_node { > protected: > - if_node() : container_node(NT_IF, NST_LIST), cond() {}; > + if_node() : container_node(NT_IF, NST_LIST), cond(), has_else(false) {}; > public: > value *cond; // glued to pseudo output (dst[2]) of the PRED_SETxxx > > virtual bool accept(vpass &p, bool enter); > > + bool has_else; > friend class shader; > }; > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev