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

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 7 01:02:43 PDT 2020


efriedma 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.
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88921



More information about the llvm-commits mailing list