[PATCH] D115615: [MemCpyOpt] Make capture check during call slot optimization more precise

Tue Dec 21 11:38:52 PST 2021

nikic added a comment.

In D115615#3205077 <https://reviews.llvm.org/D115615#3205077>, @reames wrote:

> I don't see the original bug your fixing in the test cases, and your explanation isn't clear to me.  Can you expand on that point a bit?

The referenced bug is the TODO on the `@test_bitcast` testcase.

> On your extension, I think there might be a useful generalization here.  As near as I can tell, your bit of code is effectively a local no-capture analysis.  Your reasoning could be phrased as "if the captured memory can't be accessed in a well defined manner before the end of the lifetime of the captured storage, it can't actually have been captured".  Right?
>
> Assuming that's correct, what about doing this in DSE and annotating the call param no capture directly?  Shouldn't the backwards walk DSE does on dead allocas be enough to annotate these cases?  If so, MemCpyOpt could then simply fix the bug, and we could get generally strongly nocapture reasoning everywhere.

Yeah, that sounds about right. However, I don't think that this would allow us to place a nocapture attribute: After all, the pointer may indeed be captured, it's just that the capture ends up not being used before the object goes out of scope. The capture in the call may be relevant independently of effects it may allow after the call.

================
Comment at: llvm/lib/Transforms/Scalar/MemCpyOptimizer.cpp:966
+        if (II->getIntrinsicID() == Intrinsic::lifetime_end &&
+            II->getArgOperand(1)->stripPointerCasts() == srcAlloca)
+          break;
----------------
reames wrote:
> I believe you need to check the size of the end here as well.  You could have a zero sized end, which I believe is a noop.  
Yeah, you're right.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115615