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

[Mahesha S via Phabricator via cfe-commits]

hsmhsm added inline comments.


================
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:
> hsmhsm wrote:
> > 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
> > ```
> I meant where in the clang code are these emitted, and how is that I set point found?
I understand your question as - where exactly is code emitted? 

It is Clang Codegen part, the Clang Codegen maintains a builder (function specific?) which builds and emits code, and builder always maintains default current insertion position, and it can also be asked to insert at some other place by calling the api SetInsertPoint().

In this particular case, the addrespace casts are  emitted at (by calling the builder) https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/TargetInfo.cpp#445

All I am doing here is - direct the builder to insert addrespace casts just after all static allocas by appropriately setting the insert position via SetInsertPoint().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257