Renato, On Mar 17, 2011, at 3:25 AM, Renato Golin wrote:> could help with the verification process (since it's much better to > fail verification than to fail gdb testuite), but I don't know the > design decisions being taken for debug information/metadata, and they > change too frequently to dig the code to learn.I think you are mistaken here. I maintain and support debug info for two front ends (llvm-gcc and clang). Go ahead and check svn archives for last one year and see how many times I had to update llvm-gcc FE.> There is no API > documentation and the interface (IR metadata) docs are old and > inaccurate. > > I'd say, in order of importance, the three things that need to be done ASAP are: > > 1. Stick to one representation and document it (like LangRef), so > other people could helpIn last 5 or so llvm releases, encoded debug info representation in llvm IR has changed only once (using metadata, instead of global variables). All other changes are incremental *and* backward compatible. Regarding documentation, it is on my list. However, your argument has same disconnect as some one who looks at LangReg and says I do not know what exactly FE has to generate to produce a working program. Well, what you need is a How To Write a Front End document.> 2. Enhance Validate() methods to be extremely strict (like Module's), > so it fails straight awaySee my response regarding Verify().> 3. Create tests (unit and regression) and run them during check-all, > so we don't regressI have already mentioned debuginfo-tests at least once to you earlier.> > The tests are last because it's much easier to catch an assertion than > a silent codegen error. >- Devang
On 17 March 2011 13:48, Devang Patel <dpatel at apple.com> wrote:> I think you are mistaken here. I maintain and support debug info for two front ends (llvm-gcc and clang). Go ahead and check svn archives for last one year and see how many times I had to update llvm-gcc FE.Hi Devang, First, I'm not attacking anyone. I said before and will say again: the work you've done is great. I know how complex it is to build something stable and keep it that way, and my comments were about how hard it was for me to help you in that matter. Take my last patch on Dwarf. I've run the tests, added my own, tested on a Mac and still, we found a problem only after the commit. I'm not saying that things like this won't happen, but it was really hard for me to test it and make sure the patch would actually work.> In last 5 or so llvm releases, encoded debug info representation in llvm IR has changed only once (using metadata, instead of global variables). All other changes are incremental *and* backward compatible.Not entirely true. The metadata style is the same, but the mechanism used to build it (DIBuilder) was changed (instead of DIFactory) without warning in Clang. That, per se, wouldn't be a problem, if the metadata generated by both of them were identical, which they were not. As I said before, on December we merged the LLVM tree and it broke our debug generation. It took me until February to be able to have time to fix it, but when I did, lots of arguments were different. Some had their "file" nulled, some integer arguments became boolean (or vice-versa), and some new arguments appeared out of the blue. However, migrating to DIBuilder took me only a couple of days and everything went back to normal again. So, while the infrastructure actually worked in the end, it took me by surprise and I had to guess what was going on to fix it properly.> Regarding documentation, it is on my list. However, your argument has same disconnect as some one who looks at LangReg and says I do not know what exactly FE has to generate to produce a working program. Well, what you need is a How To Write a Front End document.The debug documentation is not up-to-date. Metadata generated by DIBuilder doesn't look like what's in the docs. And the document describes some types and declarations, but it doesn't explain the relationship between them and also doesn't describe all of them. So, different from LangRef, the debug doc is not a spec. It's just a document. I tried to write how to write a front-end for Dwarf (wiki), but as I didn't have enough knowledge on how to really use it, I couldn't go too far.>> 2. Enhance Validate() methods to be extremely strict (like Module's), >> so it fails straight away > > See my response regarding Verify().So, IR has a natural verification process, while you're building it. Lots of assertions will prevent you from building rubbish, and that makes up for the lack of information in LangRef. After building IR, the validation process will catch up most of what was left over and only a few bugs slip through to the codegen process, which also has loads of assertions. So, the amount of bugs that get through to execution time are as low as possible. But it's way harder to verify Metadata, because of it's inherent variant nature. I get that and am NOT asking for a magic wand (though, if you have it... ;). And Dwarf also doesn't help, because there is a lot you can do with Dwarf that is legal but won't amount to anything in a debugger. What I'm proposing is a simple rule-set, enforced by a validation pass, that will reject dubious metadata. We could start as an optional pass, being very restrictive and failing most known code and unit tests. With time, we can extend and add corner cases to this validation until we're comfortable and turn it on by default. I personally think that it's much easier to relax strict asserts than to rely on gdb for testing. cheers, --renato
On Mar 17, 2011, at 7:29 AM, Renato Golin wrote:> On 17 March 2011 13:48, Devang Patel <dpatel at apple.com> wrote: >> I think you are mistaken here. I maintain and support debug info for two front ends (llvm-gcc and clang). Go ahead and check svn archives for last one year and see how many times I had to update llvm-gcc FE. > > Hi Devang, > > First, I'm not attacking anyone.I understand. But you're missing the point of my comment :) If the IR used to encode debug is changing rapidly, as you say, then I'd be force to frequently modify llvm-gcc FE. However, I have not modified llvm-gcc FE in last year or so, so I'd say the encoded IR has been stable. In last 6+ months, llvm-gcc build bot running gdb testsuite is consistently reporting same number of passes and fails (if you ignore inherent gdb testsuite stability issues).> I said before and will say again: the > work you've done is great. I know how complex it is to build something > stable and keep it that way, and my comments were about how hard it > was for me to help you in that matter. > > Take my last patch on Dwarf. I've run the tests, added my own, tested > on a Mac and still, we found a problem only after the commit. I'm not > saying that things like this won't happen, but it was really hard for > me to test it and make sure the patch would actually work.In other words, someone is changing target independent code generation and expects llvm regression tests to catch all bugs. If that's true, we don't need any build bots linking and executing and running llvm generated code.> >> In last 5 or so llvm releases, encoded debug info representation in llvm IR has changed only once (using metadata, instead of global variables). All other changes are incremental *and* backward compatible. > > Not entirely true. The metadata style is the same, but the mechanism > used to build it (DIBuilder) was changed (instead of DIFactory) > without warning in Clang. That, per se, wouldn't be a problem, if the > metadata generated by both of them were identical, which they were > not.Again, you're mistaken. llvm-gcc and dragon-egg still uses DIFactory and debug info quality has remained same. This says the IR used to encode debug has not been impacted by DIBuilder vs. DIFactory. Note, DIBuilder etc.. are utilities sued to produce IR, not the interface defined by the IR. In other words, replacement of of OldIRBuilder interface with NewIRBuilder has nothing to do with stability of llvm IR documented by LangRef.html.> What I'm proposing is a simple rule-set, enforced by a validation > pass, that will reject dubious metadata. We could start as an optional > pass, being very restrictive and failing most known code and unit > tests. With time, we can extend and add corner cases to this > validation until we're comfortable and turn it on by default. I > personally think that it's much easier to relax strict asserts than to > rely on gdb for testing.dwarfdump --verify will do this. - Devang