[PATCH] D84630: [StackSafety] Skip ambiguous lifetime analysis

[Chuanqi Xu via Phabricator via llvm-commits]

ChuanqiXu added inline comments.


================
Comment at: llvm/lib/Analysis/StackLifetime.cpp:78
+        continue;
+      const AllocaInst *AI = llvm::findAllocaForValue(II->getArgOperand(1));
+      if (!AI) {
----------------
vitalybuka wrote:
> ChuanqiXu wrote:
> > Maybe we can't use `findAllocaForValue` to find alloca for lifetime marker. Here I find a pattern:
> > ```
> > a = alloca struct ...
> > b = getelementptr from a
> > lifetime start (b)
> > /// ...
> > lifetime end (b)
> > ```
> > And the code here would treat the lifetime span of b, which is part of a, as the whole lifetime of a. But the lifetime span of other component of a may not be the same with the lifetime span of b. I think it may cause mismatch.
> Is this theoretical or something produces such patterns?
> I guess we will need to fallback to HasUnknownLifetimeStartOrEnd if life does not cover entire alloca.
I don't find theoretical who produces this pattern. I find such pattern by using StackLifetime in practice. The pattern seems like related to Coroutine Elision which would put one Coroutine Frame structure into another Coroutine Frame. But I don't sure if there is any other situation where would produce similar patterns.
My local fix for this problem is:
replace 
```
const AllocaInst *AI = llvm::findAllocaForValue(II->getArgOperand(1));
```
into
```
auto *OpInst = dyn_cast<Instruction>(II->getOperand(1));
auto *AI = dyn_cast<AllocaInst>(OpInst->stripPointerCasts());
```
which would fallback to HasUnknownLifetimeStartOrEnd if lifetime marker doesn't cover entire alloca as you say.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84630