aprantl added a comment. In https://reviews.llvm.org/D38042#875734, @chandlerc wrote:
> In https://reviews.llvm.org/D38042#875441, @aprantl wrote: > > > In https://reviews.llvm.org/D38042#875418, @chandlerc wrote: > > > > > Absolutely. I think the verifier should never, under any circumstances, > > > mutate the IR. Think about it, with the current design if a pass corrupts > > > the debug info the verifier may "hide" this by stripping it out rather > > > than allowing us to find it. > > > > > > Okay, I think I agree. Before I venture off implementing this, do you think > > that separating out a StripBrokenDebugInfoPass that depends on the Verifier > > is right way forward? > > > I don't know what you mean by "depends on" and I'm not sure I'm the right > person to say what the exact best design is... But I'd throw something > together and start that review on llvm-commits where we can get folks more > familiar w/ these details involved to figure it out. It at least seems to be > in the right direction. > > My only concern is that passes are supposed to be able to rely on the > verifier passing, and this wouldn't... Not sure how to handle that case. > > > > > > >> But auto-upgrade happens on *read*. If you want to add the debug info > >> stripping to auto-upgrade, that's a reasonable discussion to have, and no > >> change here will be required. There might be concerns about that on the > >> LLVM side, I don't know. But the verifier (as well as running it here) > >> does not seem like the right solution. > > > > Would splitting the VerifierPass into a VerifierPass and a > > StripBrokenDebugInfoPass *together* with adding a -enable-llvm-verifier (an > > explicit opposite of -disable-llvm-verifier) work for you? > > If you want a way to use the `clang` binary to strip broken debug info from a > bitcode input, I would add a flag that does exactly this, and leave the > verifier as an independent component. I don't know whether such a flag makes > sense or not (Richard or other more deep in Clang-land would have a better > feel than I would). > > But I think that whether the verifier is enabled or not should be orthogonal > from any debug info stripping. Stripping the debug info might impact whether > something verifies, but the flags should be completely independent. > > However, if the debug info stripping ends up as part of auto upgrade, all of > this becomes much simpler to think about. I like the idea of making debug info stripping a part of the auto upgrade process, but in order to do this, we would need to run the verifier as part of the auto upgrade process (probably inside `llvm::UpgradeDebugInfo()`) in order to determine that stripping is necessary. Do you see any problems with that? I guess as long as we wire up clang's --disable-llvm-verifier option to the bitcode reader we can still get the current behavior if somebody really wants that. I will explore this idea some more. Thanks for your input so far! https://reviews.llvm.org/D38042 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
