[PATCH] D28170: [LICM] When promoting scalars, allow inserting stores to thread-local allocas

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: llvm/trunk/lib/Transforms/Scalar/LICM.cpp:1037
     SafeToInsertStore =
-        isAllocLikeFn(Object, TLI) && !PointerMayBeCaptured(Object, true, true);
+        (isAllocLikeFn(Object, TLI) || isa<AllocaInst>(Object)) &&
+        !PointerMayBeCaptured(Object, true, true);
----------------
efriedma wrote:
> trentxintong wrote:
> > The question I really want to ask is whether the check for MayNotBeCaptured is necessary for Alloca. Should not alloca be implicitly thread local ? The language reference does not explicitly state it.
> In general, memory allocated using alloca can be used in all the same ways you could use memory allocated with malloc().  Not sure what sort of restriction you're expecting.
(Slight digression) There is one difference -- alloca memory is inherently frame local.  This means you don't have to worry about the memory escaping via a `ret`.  DSE handles a specific instance of this, where the stores are in the return block itself:

```
i8* @f() {
  %t = alloca i32
  store i32 42, i32* %t
  ret i32* %t
}
```

but e.g. slightly more complex cases like

```
define i32* @f(i1* %cond) {
  %t = alloca i32
  store i32 42, i32* %t
  %c = load volatile i1, i1* %cond
  br i1 %c, label %a, label %b

a:
  %c2 = load volatile i1, i1* %cond
  ret i32* %t

b:
  ret i32* null
}
```

are not handled by -O3.


Repository:
  rL LLVM

https://reviews.llvm.org/D28170