[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