On Sun, Jul 09, 2017 at 05:19:27PM -0700, Christopher Li wrote: > On Sun, Jul 9, 2017 at 9:52 AM, Luc Van Oostenryck > <luc.vanoostenryck@xxxxxxxxx> wrote: > > > > It could have made a difference in the test suite because killing > > unreachable code can create new simplification opportunities. > > Yes. As far as I can tell, when the kill instruction get invoked, it should > have cause the REPEAT_CSE. "Kill unreachable bb" shouldn't need > to set CSE flag. > > > What do you mean exactly by 'pass full kernel check'? That you get > > exactly the same warnings with & without the patch? > > Yes, that is what I usually do to give sparse some workload other > than the test-suite. OK. > > It would also be nice to know why the patch that came with the bug > > report have been discarded without a single comment. > > Sorry I haven't been more specific. I think the original patch is a bit > too complicated. Also just as I suspected, there is more than the bb > list get affected. If possible, I think we should try to avoid get into nested > loop delete in the first place rather than make the nested loop delete > work. I can hardly qualify my patch as 'complicated' but well ... This patch have been done with two goals in mind: 1) solve the problem with the deleted BB 2) do not change anything at the simplification since interaction between differents parts there can have bad effects (it's to fix a problem there that the kill_unreachable_bbs() had been added). Your patch, take care of point 1) but not 2) and effectively change when the BBs are removed and this change the interaction between insert_branch() and some others parts. I have given a run of some of my tests I run here and there is indeed some changes. Most of these changes are obviously correct. Some are not obvious at all and would need to be investigated. Some look scary (and of course would need to be investigated too). In short, I can't guarantee that the "crazy programmer" problem won't come back or that a similar one haven't been created. > Also just as I suspected, there is more than the bb > list get affected. Sure, but this have nothing to do with the choice of patches here since your pacth also just tak ecare of the deleted BBs from the inner kill_unreachable_bbs(). > If possible, I think we should try to avoid get into nested > loop delete in the first place rather than make the nested loop delete > work. This seems an obvious and easy way to avoid problems but this won't something that will be sustainable. It would put a huge constraint on what can be done and what can't done, which is why, of course, I earlier suggested to instead change the way elements are 'deleted' (like instructions are already never removed from their list but just marked as deleted). -- Luc -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
