This is an archive of the discontinued LLVM Phabricator instance.

Mark MergedLoadStoreMotion as not preserving MemDep results
ClosedPublic

Authored by dotdash on Feb 12 2018, 3:04 AM.

Details

Summary

MemDep caches results that signify that a dependence is non-local, and
there is currently no way to invalidate such cache entries.
Unfortunately, when MLSM sinks a store that can result in a non-local
dependence becoming a local one, and then MemDep gives wrong answers.
The easiest way out here is to just say that MLSM does indeed not
preserve MemDep results.

Diff Detail

Repository
rL LLVM

Event Timeline

dotdash created this revision.Feb 12 2018, 3:04 AM

I think this Is in line with what Gerolf had in mind, but I'll let him sign off this one.

Hi Gerolf,

I just wanted to check if you could have a look at this patch.

Thanks!

Yes, this LGTM. I could not find a way to perform updates properly in the example. If recomputing the dependencies results in a compile-time issue we will have to dig deeper into MemDep.

Thanks for your fix + patience!

Cheers
Gerolf

This revision was not accepted when it landed; it landed in state Needs Review.Feb 23 2018, 2:44 AM
This revision was automatically updated to reflect the committed changes.