rjmccall 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:
> 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.
Yes, my point was about PHIs and any other instruction that might be required
to appear at the start of a block.
https://reviews.llvm.org/D27680
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
