[PATCH] D136285: Bad optimization with alloca and intrinsic function stackrestore

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

See https://github.com/llvm/llvm-project/issues/47943 for some discussion on the topic, in particular also the last comment. I doubt that dropping the isStaticAlloca() check *just* here is the right thing to do -- we need to have some kind of self-consistent notion of what a static alloca is. The current notion appears to be that any (fixed-size, non-inalloca) alloca in the entry block is a static alloca, regardless of where exactly it appears, and as such really is not clobbered by stackrestore. Of course, codegen needs to be consistent with that notion. If we want to change that, I believe this needs to actually happen inside isStaticAlloca(), so everything has a consistent picture about this.

Regarding the tail marker, I believe that should be fixed inside markTails() -- not accessing stack memory is a prerequisite for the `tail` marker, so inferring it for `stackrestore` seems incorrect to me, independently of the other question.



================
Comment at: llvm/test/Transforms/MemCpyOpt/stackrestore.ll:3
 ; RUN: opt -S -memcpyopt < %s -verify-memoryssa | FileCheck %s
+; RUN: opt -S -passes=memcpyopt -verify-memoryssa < %s | FileCheck %s --check-prefix=CHECK-TEST2
 
----------------
The second run line is unnecessary. The check lines should be generated using update_test_checks.py.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136285