dblaikie accepted this revision. dblaikie added a comment. In D136041#3864171 <https://reviews.llvm.org/D136041#3864171>, @yonghong-song wrote:
> In D136041#3863748 <https://reviews.llvm.org/D136041#3863748>, @dblaikie > wrote: > >> Hmm - this does mean linking IR can produce invalid code, though, right (you >> link in a definition of the function, so what was valid is now invalid - >> because it now has a definition, can be inlined, etc)? Is that new? >> concerning? > > @dblaikie do you have a concrete example to illustrate the problem? One translation unit with a call to a function declaration, that call has no !dbg attachment. Another translation unit with the definition of that declared function. As-is, this would not cause any verifier error or backend debug info assertion. Link the two together and now it's a verifier error - and, depending on the contents of the function definition, maybe a backend debug info assertion. In D136041#3864782 <https://reviews.llvm.org/D136041#3864782>, @eddyz87 wrote: > In D136041#3863748 <https://reviews.llvm.org/D136041#3863748>, @dblaikie > wrote: > >> Hmm - this does mean linking IR can produce invalid code, though, right (you >> link in a definition of the function, so what was valid is now invalid - >> because it now has a definition, can be inlined, etc)? Is that new? >> concerning? > > As far as I understand this > <https://lists.llvm.org/pipermail/llvm-commits/Week-of-Mon-20160425/350532.html> > discussion the check in question is "best effort". Fair point > My change further narrows conditions when verifier would report an error, > thus it should not add any new failures to the existing code. No, the opposite. > But yes, hypothetically there might be a situation when old version of the > check would have caught a miss-behaving transformation at an earlier stage > (before linking IR rather then after). Right - now things that were invalid categorically, are now valid, unless you link them. Which is a bit tricky. Ah well, as you say - it's best effort. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136041/new/ https://reviews.llvm.org/D136041 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
