Page MenuHomePhabricator

[LoopPred] Fix two subtle issues found by inspection
ClosedPublic

Authored by reames on Thu, Oct 31, 5:14 PM.

Details

Summary

This patches fixes two issues noticed by inspection when going to enable the loop predication code in IndVarSimplify. Arguably, these could be split. If reviewers want me to do so, I'm happy to do so.

Issue 1 - Both the LoopPredication transform, and the already on by default optimizeLoopExits transform, modify the exit count of the exits they modify. (either to 0 or Infinity) Looking at the code more closely, this was not reflected into SCEV and we were instead running later transforms with incorrect SCEVs. Fixing this requires forgetting the loop, weakening a too strong assert, and updating SCEV to not pessimize results when a loop is provable untaken. I haven't been able to find a test case to demonstrate the miscompile.

Issue 2 - For modules without a data layout, we can end up with unsized pointer typed exit counts. Just bail out of this case.

I think these are the last two issues which need addressed before we enable this by default. The code has already survived a decent amount of fuzzing without revealing either of the above.

Diff Detail

Event Timeline

reames created this revision.Thu, Oct 31, 5:14 PM
nikic accepted this revision.Wed, Nov 6, 11:54 AM

LGTM

This revision is now accepted and ready to land.Wed, Nov 6, 11:54 AM
This revision was automatically updated to reflect the committed changes.