[PATCH] D88921: [MemCpyOpt] Fix thread-safety of call slot opimization

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:810
+  MemoryLocation destLoc(cpyDest, srcSize);
+  if (isModOrRefSet(AA->callCapturesBefore(C, destLoc, DT))) {
+    // Ensure dest is not accessed by the call.
----------------
efriedma wrote:
> nikic wrote:
> > efriedma wrote:
> > > callCapturesBefore only checks the call C itself.  Don't we also care if the pointer is captured somewhere before the call?
> > I find this method to be somewhat confusingly named, but what it actually does is to a) check for captures up to and including the call (-> ModRef) and b) a potential access via a non-capturing argument in the call itself.
> > 
> > See https://github.com/llvm/llvm-project/blob/334ec6f807fa65e09571fa42a0c3be0eb39e7c0f/llvm/lib/Analysis/AliasAnalysis.cpp#L652.
> That makes sense.
> 
> That said, I don't think the handling of noalias arguments here is correct.  In general, noalias only affects memory accesses that dynamically execute.  So here, if the write to destLoc is dynamically dead, you can't do anything useful with the noalias attribute; you have to treat it like any other captured pointer.
> 
> Not sure if that's a bug in callCapturesBefore(), or the way it's used here.
> That said, I don't think the handling of noalias arguments here is correct. In general, noalias only affects memory accesses that dynamically execute. So here, if the write to destLoc is dynamically dead, you can't do anything useful with the noalias attribute; you have to treat it like any other captured pointer.

But doesn't (a non-captured) `noalias` imply that a write also cannot happen from a different thread, as that means that a write to the noalias object occurred without going through the noalias pointer? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88921/new/

https://reviews.llvm.org/D88921