On Tue, Jul 23, 2019 at 5:58 PM Francisco Jerez <curroje...@riseup.net> wrote: > > Specifically the optimization of a conditional BREAK + WHILE sequence > into a conditional WHILE seems pretty broken. The list of successors > of "earlier_block" (where the conditional BREAK was found) is emptied > and then re-created with the same edges for no apparent reason. On > top of that the list of predecessors of the block immediately after > the WHILE loop is emptied, but only one of the original edges will be > added back, which means that potentially several blocks that still > have it on their list of successors won't be on its list of > predecessors anymore, causing all sorts of hilarity due to the > inconsistency in the control flow graph.
It's been ~5 years since I wrote the code, but I think the idea was to prevent ::combine_with from ever combining blocks that had "internal" edges -- that is, the first block cannot have outgoing edges and the second block cannot have incoming edges. That still seems to me to be a good idea. I guess it would be fine if that were relaxed to be the only The intention was to ensure that in the rare cases that we use ::combine_with that the programmer has to think hard about how they're rewiring the CFG. > The solution is to remove the code that's removing valid edges from > the CFG. cfg_t::remove_block() will already clean up after itself. > The assert in bblock_t::combine_with() also needs to be removed since > we will be merging a block with multiple children into the first one > of them. Okay, so I think you're saying that ::remove_block already does exactly what we need so we don't need to bother ensuring that the "internal" edges are removed. If that's an accurate restatement of your claim then this patch makes sense to me and is Reviewed-by: Matt Turner <matts...@gmail.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
