jdoerfert added inline comments.
================ Comment at: clang/lib/CodeGen/CGExpr.cpp:151 + // allocas. + Builder.CreateStore(Init, Var); } ---------------- 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; } ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110257/new/ https://reviews.llvm.org/D110257 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
