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

[fhahn]

Summary

This updates the MemorySSA backed implementation to treat arguments
passed by value similar to allocas: in they are assumed to be invisible
in the caller. This is similar to how they are treated in legacy DSE.

[efriedma]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1568

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.

[efriedma]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1566–1572

Is there an incorrectly resolved merge conflict here?

[fhahn]

Remove unintended duplication, limit InivisibleBeforeRet to byval.

[fhahn]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1566–1572

Yes that was unintentional. sorry about that!

1568

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.

[efriedma]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1568

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

[fhahn]

Update comment as suggested, thanks Eli!

[fhahn]

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

1568

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.

[efriedma]

LGTM

llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp

for (BasicBlock *BB : post_order(&F)) {
           }
         }
       }
     }
 
     // Treat byval or inalloca arguments the same as Allocas, stores to them are
     // dead at the end of the function.
     for (Argument &AI : F.args())
-      if (AI.hasPassPointeeByValueAttr())
+      if (AI.hasPassPointeeByValueAttr()) {
+        // For byval, the caller doesn't know the address of the allocation.
+        if (AI.hasByValAttr())
           State.InvisibleToCallerBeforeRet.insert(&AI);
+        State.InvisibleToCallerAfterRet.insert(&AI);
+      }
+
     return State;
   }
 
   Optional<MemoryLocation> getLocForWriteEx(Instruction *I) const {
     if (!I->mayWriteToMemory())
       return None;
 
     if (auto *MTI = dyn_cast<AnyMemIntrinsic>(I))