[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:151
+  // allocas.
+  Builder.CreateStore(Init, Var);
 }
----------------
jdoerfert wrote:
> hsmhsm wrote:
> > jdoerfert wrote:
> > > I'm not even sure this is necessarily correct. 
> > > 
> > > How do we know the new store is not inside a loop and might write the value more than once, potentially overwriting a value set later?
> > > 
> > > Aside from that (important) question, you need to update the documentation of the function. It doesn't correspond to the new semantics anymore.
> > > 
> > I am not convinced with above comments -Because, this code neither changes the address nor the initializer (https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CodeGenFunction.h#L2548), but only a place where the initialization happens.
> > 
> > Further, as the documentation says, the initializer must be a constant or function argument (and surprisingly, I do not see any assertion about it). Hence, even if the initialization happens within the loop, loop invariant code motion pass should detect it.
> > 
> > That said, if we think, it is better to keep the initialization within entry block, we can do it at the end of the block.
> What the initial value is is irrelevant. Arguing LICM should take care of it is also not helpful. Changing the location of the initialization is in itself a problem:
> 
> Before:
> ```
> a = alloca
> a = 0;                // init is in the entry block
> for (...) {
>   use(a);
>   a = a + 1;
> }
> ```
> 
> After, potentially:
> ```
> a = alloca
> for (...) {
>   a = 0;              // init is now at the builder insertion point
>   use(a);
>   a = a + 1;
> }
> ```
Clarification - I am not trying to argue anything here with anybody, I am trying to defend myself, where I feel I am right as per my knowledge. If I realize that I am at false later, then I correct myself, but I am not arguing, and will never ever do.

Ok, I will try to fix it from the practical point of view.

But,  I still think as follows - From theoretical/good programming/good front-end code generation perspective, initialization of something will never happen within construct like loop, otherwise initialization has no meaning at all.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110257