[PATCH] D82222: [DSE, MSSA] Treat passed by value args as invisible to caller.

Sun Jun 21 04:45:36 PDT 2020

fhahn marked an inline comment as done.
fhahn added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1584
+        State.InvisibleToCallerBeforeRet.insert(&AI);
+        State.InvisibleToCallerAfterRet.insert(&AI);
+      }
----------------
efriedma wrote:
> fhahn wrote:
> > efriedma wrote:
> > > For inalloca and preallocated, the argument has the same address in the caller and the callee.  So if I'm understanding correctly, InvisibleToCallerBeforeRet should only apply specifically to byval.
> > Right, I thought the caller cannot expect to observe any writes to any PassPointeeByVal argument, but it is not entirely clear from the langref. I've updated the code as suggested. Legacy DSE seems to treat them at least as if writes are not observable after the function returns.
> For all of byval, inalloca, and preallocated, the allocation is implicitly freed when the function's stack frame is freed (when the function returns or unwinds).
> 
> The aspect where inalloca and preallocated differ is related to escaping.  Essentially, for inalloca/preallocated arguments, the address has already escaped before the function starts executing.
> 
> Not sure what "even if the function exits through an exception" means in this context.  The comment should be something more like "for byval, the caller doesn't know the address of the allocation".
> 
Makes sense, I've updated the comment, thanks!

Currently the InvisbleToCallerSets are framed as the object being invisible to the caller, but I think the key point is whether modifications in the callee are visible to the caller. I think then it would be possible to include inalloca & preallocate as invisible before returning (i.e. when unwinding). But that's potential for another patch.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82222