[PATCH] D53596: [ThinLTO] Fix a crash in lazy loading of Metadata

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 14 11:00:50 PST 2018


dexonsmith resigned from this revision.
dexonsmith added a comment.

In https://reviews.llvm.org/D53596#1298787, @dexonsmith wrote:

> Sorry, I missed your questions.  Thanks for pinging.
>
> I wasn’t concerned about this patch going in.  I was just concerned we may have had compile time regressions already.  Metadata forward references are quite expensive to track and resolve.
>
> But I think I’d misread the patch entirely.  Is see this is just loosening to Metadata .


I misread the code in a different way.  I don’t have time this week to really understand this so I’m resigning as a reviewer.  I don’t want to hold up the fix and Steven already reviewed this.

> - Am I correct that somehow the scope field is not an MDNode?  The verifier should fail for that, unless something has changed.  @aprantl can you take a quick look?
> - Can we have a positive test for what the debug info should like like here, rather than just a crash test?

I’m still a bit concerned about the test being debug info related and not testing the debug info output at all, but if @aprantl has comments there they can probably be dealt with post-commit.


Repository:
  rL LLVM

https://reviews.llvm.org/D53596





More information about the llvm-commits mailing list