This is an archive of the discontinued LLVM Phabricator instance.

[GVNHoist] safeToHoistLdSt incorrectly checks whether a defining access dominates the insertion point
ClosedPublic

Authored by labrinea on Jul 19 2018, 9:40 AM.

Details

Summary

Bug fix for PR36787. The regression test is a reduced version of the original reproducer attached to the bug report. When reasoning if it's safe to hoist a load we want to make sure that the defining memory access dominates the new insertion point of the hoisted instruction. safeToHoistLdSt calls firstInBB(InsertionPoint,DefiningAccess) which returns false if InsertionPoint == DefiningAccess, and therefore it falsely thinks it's safe to hoist.

Diff Detail

Event Timeline

labrinea created this revision.Jul 19 2018, 9:40 AM
sebpop accepted this revision.Jul 20 2018, 6:17 PM

looks good, thanks!

This revision is now accepted and ready to land.Jul 20 2018, 6:17 PM
This revision was automatically updated to reflect the committed changes.