Page MenuHomePhabricator

MemCpyOpt does not preserve MemDep, unforunately. See PR 36965, 36964, 36940, 36944, 36943, and 36941 Testcases coming.
AcceptedPublic

Authored by dberlin on Apr 3 2018, 4:37 PM.

Details

Reviewers
davide
hfinkel
Summary

There are a number of causes here:

  1. Same as pr36063, change in dependence type from local to nonlocal,

or vice versa, can't be invalidated in the cache.

  1. MemDep does not do transitive cache invalidation, so

loads that would now point to a store/memcpy/etc that it has inserted,
will still point to the wrong place.

  1. An unknown cause i'm still tracking down that is causing preservation failure

even when memcpyopt does nothing!

The TL;DR is that anything that moves stores across blocks probably should not be preserving memdep
(or is most likely broken right now), a least until it can be fixed.

Diff Detail

Event Timeline

dberlin created this revision.Apr 3 2018, 4:37 PM

Updating D45238: MemCpyOpt does not preserve MemDep, unforunately. See PR 36965, 36964, 36940, 36944, 36943, and 36941

Testcases coming.

davide added a comment.Apr 3 2018, 4:47 PM

yes, I've been following this (and I agree with you we should really disable the preservation).
Do we happen to have some tests lying around we can use? I think Zhendong generated many, but I haven't really checked whether they can be used.

davide accepted this revision.Apr 3 2018, 4:48 PM

Looks like there was a race condition. This LGTM with the test cases added.

This revision is now accepted and ready to land.Apr 3 2018, 4:48 PM