[PATCH] D109917: [InstCombine] Improve TryToSink for side-effecting calls that would be trivially dead

Mon Dec 13 10:41:44 PST 2021

anna added a comment.

In D109917#3188468 <https://reviews.llvm.org/D109917#3188468>, @nikic wrote:

> I see that you've added the writeonly argument handling to this patch. I would  prefer to split that off into a separate one, because it's not directly related. Even if it means that this lands without any upstream test changes at first.

I don't think we generally do this. I will need to add either the allocations or the paramcase soon-ish after this. But I see your point (since the change as-is does not affect any upstream code). Will wait if @reames has any concerns.

> I think something that your reasoning on writeonly arguments may be missing is a capture by the call, in which case there might be an indirect read of the alloca later that is not covered by your analysis. Might not matter for the sinking as implemented here, but would make it illegal to drop the call.

That's true. Note that the capture should be by another argument passed to the call since the called function is argmemonly.

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

https://reviews.llvm.org/D109917