ahatanak added inline comments.
================
Comment at: lib/CodeGen/CGDecl.cpp:917
+ if (!InsertPt)
+ Builder.SetInsertPoint(BB, BB->begin());
+ // If InsertPt is a terminator, insert it before InsertPt.
----------------
ahatanak wrote:
> rjmccall wrote:
> > BB->begin() is not necessarily a legal place to insert a call. This could
> > happen if e.g. a scope was unconditionally entered after a statement
> > including a ternary expression.
> >
> > Also, I think lexical scopes don't necessarily have an active basic block
> > upon entry, because their entry can be unreachable. This happens most
> > importantly with e.g. switch statements, but can also happen with goto or
> > simply with unreachable code (which it's still important to not crash on).
> I'm not sure why BB->begin() wouldn't be the right place to insert a call
> when a scope is entered after a ternary expression. Is it because
> EmitConditionalOperatorLValue inserts a phi at the beginning of block
> "cond.end"?
>
> Also, I'm not sure when goto or unreachable code might be mis-compiled. Could
> you elaborate further on that? I can see now why lifetime.start can be
> inserted at a wrong location when lowering switch statements because
> EmitSwitchStmt creates and inserts a SwitchInst before a lexical scope is
> entered, but I'm not sure about the other two cases you mentioned.
Also, I realized this patch doesn't always insert lifetime.start at the
beginning of the block the variable is associated with. For example, when the
following code is compiled, lifetime.start for "i" is inserted after the call
to foo2, but it should be inserted before the call (at the beginning of the
function):
```
void foo1(int a) {
foo2();
for (int i = 0; i < 10; ++i)
foo3();
}
```
I'll fix this in my next patch.
https://reviews.llvm.org/D27680
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
