ping...
On Wed, 4 Sep 2013 18:45:39, Bernd Edlinger wrote: > > On Tue, 3 Sep 2013 12:31:50, Richard Biener wrote: >> On Fri, Aug 30, 2013 at 6:43 PM, Bernd Edlinger >> <bernd.edlin...@hotmail.de> wrote: >>> Now I think this is good opportunity for a simple heuristic: >>> >>> If a statement is at loop level 1 we can move it out of the loop, >>> regardless of the fact, that it may invoke undefined behavior, because if >>> it is >>> moved then out of any loops, and thus it cannot be an induction variable and >>> cause problems with aggressive loop optimizations later. >> >> VRP can still cause wrong-code issues (it's probably hard to generate a >> testcase though). Also a less conservative check would be to see >> whether we hoist _into_ loop level 0 (though we cannot check that at >> the point where you placed the check). > > Well, then I should better revert this heuristic again. > >>> Only statements with possible undefined behavior in nested loops can become >>> induction variable if lim moves them from one loop into an outer loop. >>> >>> Therefore with extremely much luck the test case will pass unmodified. >>> I tried it, and the patch passes bootstrap and causes zero regressions >>> with this heuristic. >>> >>> Ok for trunk now? >> >> Jakub mentioned another possibility - make sure the moved expression >> does not invoke undefined behavior by computing in an unsigned type. > > That is a possibility, but on the other hand, that would obscure the undefined > behavior and thus prevent other possible optimizations later. > > Another possibility would be to move the statement together with the > enclosing if-statement, thus really preserving the execution. > >> That said, for the sake of backporting we need a patch as simple as >> possible - so it would be interesting to see whether the patch without >> the loop 1 heuristic has any effect on say SPEC CPU 2006 performance. > > I do not have access to that test, but on the dhrystone benchmark this patch > has no influence whatsoever. > > Attached you'll find the latest version of my patch without the heuristic. > Bootstrapped on i686-pc-linux-gnu and regression tested again. > > Ok for trunk and 4.8 branch?