This revision was automatically updated to reflect the committed changes.
Closed by commit rL304470: [CGDebugInfo] Finalize SubPrograms when we're done
with them (authored by kfischer).
Changed prior to commit:
https://reviews.llvm.org/D33705?vs=101092&id=101107#toc
Repository:
rL LLVM
http
loladiro added a comment.
I don't think that change is entirely necessary. I don't have any strong
objections to it, but I also don't see a good reason to require it. In any
case, let me get this in to be able to re-land https://reviews.llvm.org/D33655
and we can revisit the more disruptive cha
aprantl added a comment.
Looks good.. Are you also planning to change DIBuilder to not finalize
subprograms automatically any more (and not insert them into AllSubprograms)?
(That will be the more impactful change as it will force all non-clang
frontends to make a similar change).
https://rev
loladiro added a comment.
There's already such a test case, but the cloning currently doesn't assert
properly (but it can generate incorrect code). https://reviews.llvm.org/D33655
fixes that up, so I think the testing is covered once that LLVM commit goes in.
I'll hold off a little while to giv
dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.
I guess this would need a cross-project test case (ie: it'd have to run LLVM
optimizations to fail/pass/demonstrate the fix). I think it'd be OK to add one
if there's a neat/clean/obvious
loladiro added a comment.
@aprantl @dblaikie See if you like this better.
https://reviews.llvm.org/D33705
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
loladiro updated this revision to Diff 101092.
loladiro added a comment.
Finalize all subprograms when we're done emitting them.
The one we're interested in is a side effect, but doing
this uniformly might be cleaner and help avoid similar errors in the future.
https://reviews.llvm.org/D33705
F
For some reason your comments aren't showing up on Phabricator, so replying
via email - hope everybody sees it.
If SPs are now all finalized early - I'm assuming there's some other code
> that finalizes SPs that's no longer needed?
>
Not all of them, just the one before cloning. I wanted to make
I'll let Adrian weigh in - but I suspect, if possible, it'd be better to
just finalize them all once IRGen for the function finishes.
On Tue, May 30, 2017 at 6:02 PM Keno Fischer
wrote:
> For some reason your comments aren't showing up on Phabricator, so
> replying via email - hope everybody sees
If SPs are now all finalized early - I'm assuming there's some other code
that finalizes SPs that's no longer needed?
Also - this seems like a layering/separation issue - does other code call
into the DI as casually/explicitly as this code being added to CGVTables?
(Or should the callback go throu
loladiro created this revision.
LLVM commit https://reviews.llvm.org/D33655 was reverted, because it exposed an
assertion failure,
in `GenerateVarArgsThunk`. That function attempts to clone a function
before clang is entirely done generating the debug info for the current
compliation unit. That i
11 matches
Mail list logo
