[PATCH] D110257: [CFE][Codegen] Do not break the contiguity of static allocas.

[Mahesha S via Phabricator via cfe-commits]

hsmhsm added a comment.

In D110257#3015553 <https://reviews.llvm.org/D110257#3015553>, @arsenm wrote:

> I do think it's cleaner/more canonical IR to cluster these at the top of the block, but I don't understand this comment:
>
>> otherwise, inliner's attempt to move static allocas from callee to caller will fail,
>
> The inliner successfully moves allocas to the caller's entry block, even with addrspacecasts interspersed.

The logic at https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/InlineFunction.cpp#L2093 assumes that static allocas (within callee) are contiguous.



================
Comment at: clang/lib/CodeGen/CGExpr.cpp:102-106
+      auto *EBB = AllocaInsertPt->getParent();
+      auto Iter = AllocaInsertPt->getIterator();
+      if (Iter != EBB->end())
+        ++Iter;
+      Builder.SetInsertPoint(EBB, Iter);
----------------
arsenm wrote:
> Where are the addrspacecasts inserted? Could you just adjust where those are inserted instead?
The addressspace casts are inserted immediately after all static allocas (top static alloca cluster).

An example:

Before this patch:

```
entry:
  %N.addr = alloca i64, align 8, addrspace(5)
  %N.addr.ascast = addrspacecast i64 addrspace(5)* %N.addr to i64*
  %vla.addr = alloca i64, align 8, addrspace(5)
  %vla.addr.ascast = addrspacecast i64 addrspace(5)* %vla.addr to i64*
  %a.addr = alloca i32*, align 8, addrspace(5)
  %a.addr.ascast = addrspacecast i32* addrspace(5)* %a.addr to i32**
  %vla.addr2 = alloca i64, align 8, addrspace(5)
  %vla.addr2.ascast = addrspacecast i64 addrspace(5)* %vla.addr2 to i64*
  %b.addr = alloca i32*, align 8, addrspace(5)
  %b.addr.ascast = addrspacecast i32* addrspace(5)* %b.addr to i32**
  %N.casted = alloca i64, align 8, addrspace(5)
  %N.casted.ascast = addrspacecast i64 addrspace(5)* %N.casted to i64*
  %.zero.addr = alloca i32, align 4, addrspace(5)
  %.zero.addr.ascast = addrspacecast i32 addrspace(5)* %.zero.addr to i32*
  %.threadid_temp. = alloca i32, align 4, addrspace(5)
  %.threadid_temp..ascast = addrspacecast i32 addrspace(5)* %.threadid_temp. to i32*  
  store i64 %N, i64* %N.addr.ascast, align 8
```

With this patch:

```
entry:
  %N.addr = alloca i64, align 8, addrspace(5)
  %vla.addr = alloca i64, align 8, addrspace(5)
  %a.addr = alloca i32*, align 8, addrspace(5)
  %vla.addr2 = alloca i64, align 8, addrspace(5)
  %b.addr = alloca i32*, align 8, addrspace(5)
  %N.casted = alloca i64, align 8, addrspace(5)
  %.zero.addr = alloca i32, align 4, addrspace(5)
  %.threadid_temp. = alloca i32, align 4, addrspace(5)
  %.threadid_temp..ascast = addrspacecast i32 addrspace(5)* %.threadid_temp. to i32*
  %.zero.addr.ascast = addrspacecast i32 addrspace(5)* %.zero.addr to i32*
  %N.casted.ascast = addrspacecast i64 addrspace(5)* %N.casted to i64*
  %b.addr.ascast = addrspacecast i32* addrspace(5)* %b.addr to i32**
  %vla.addr2.ascast = addrspacecast i64 addrspace(5)* %vla.addr2 to i64*
  %a.addr.ascast = addrspacecast i32* addrspace(5)* %a.addr to i32**
  %vla.addr.ascast = addrspacecast i64 addrspace(5)* %vla.addr to i64*
  %N.addr.ascast = addrspacecast i64 addrspace(5)* %N.addr to i64*
  store i64 %N, i64* %N.addr.ascast, align 8
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257