[PATCH] D88799: [MemCpyOpt] Check for throwing calls during call slot optimization

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:835
+    }
+  }
+
----------------
efriedma wrote:
> nikic wrote:
> > Replying to @efriedma's comment on https://reviews.llvm.org/D88805#inline-824472: I believe checking for throwing instructions rather than guaranteed-to-transfer is correct here, because cpyDest cannot be read between the call and the copy, as a precondition for the call-slot optimization. So even if one of the instructions does not return, an early write to cpyDest will not be observable, with two caveats:
> >  * The call itself might read cpyDest. We explicitly protect against this below (see the getModRefInfo and callCapturesBefore checks).
> >  * We might throw and the catching code might read cpyDest. That's what we protect against here.
> The case I'm concerned about is a multithreaded context.  Say there's an infinite loop between the call and the write to cpyDest.  Then the current thread never touches cpyDest at all, so some other thread could legally use the memory.
> 
> If we're sure no other thread can access the memory, ensuring we don't throw/longjmp would be sufficient, for the reasons you describe.
I think you are right, and there is nothing that prevents that from happening right now. Unfortunately this also seems like a more significant limitation (not just because of the willreturn requirement, but also because this affects captured allocas, unlike the unwinding case). For now I've added a more extensive comment listing the different cases and marked this part as a TODO.


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

https://reviews.llvm.org/D88799