[PATCH] D53362: [Prototype] Heap-To-Stack Conversion Pass

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D53362#1267978, @efriedma wrote:

> I think you have to be careful about something like the following:
>
>   void f() {
>     void *p = malloc(4);
>     nocapture_func_frees_pointer(p);
>     func_throws();
>     free(p);
>   }
>   
>
> I don't think it's possible to trigger the issue at the moment, though, because we don't have a mustreturn attribute (so isGuaranteedToTransferExecutionToSuccessor() will return false for nocapture_func_frees_pointer).


I think you're right. Thanks! -- I also rebased my nofree-attribute patch the other day. I'll post that update too. I think that can be used to solve this problem because in cases where we can infer nocapture we should also be able to infer nofree (unless, of course, as in this case, where the function actually does free the data).



================
Comment at: lib/Transforms/Scalar/HeapToStack.cpp:138
+      // data elsewhere. Currently, things that cause an infinite loop to be
+      // well defined (e.g., atomics, I/O) will also cause
+      // isGuaranteedToTransferExecutionToSuccessor to return false,
----------------
efriedma wrote:
> isGuaranteedToTransferExecutionToSuccessor is true for atomics.
Indeed (at least some of them)...

  bool llvm::isGuaranteedToTransferExecutionToSuccessor(const Instruction *I) {
    // A memory operation returns normally if it isn't volatile. A volatile
    // operation is allowed to trap.
    //
    // An atomic operation isn't guaranteed to return in a reasonable amount of
    // time because it's possible for another thread to interfere with it for an
    // arbitrary length of time, but programs aren't allowed to rely on that.
    if (const LoadInst *LI = dyn_cast<LoadInst>(I))
      return !LI->isVolatile();
    if (const StoreInst *SI = dyn_cast<StoreInst>(I))
      return !SI->isVolatile();
    if (const AtomicCmpXchgInst *CXI = dyn_cast<AtomicCmpXchgInst>(I))
      return !CXI->isVolatile();
    if (const AtomicRMWInst *RMWI = dyn_cast<AtomicRMWInst>(I))
      return !RMWI->isVolatile();
    if (const MemIntrinsic *MII = dyn_cast<MemIntrinsic>(I))
      return !MII->isVolatile();



https://reviews.llvm.org/D53362