On 24 February 2011 21:34, Jason Kim <jasonwkim at google.com> wrote:> On Thu, Feb 24, 2011 at 1:29 PM, Devang Patel <dpatel at apple.com> wrote:It simplified a bit type construction but the overall code did not reduce that much. It's still very similar to the previous style but had a few differences in the IR output. We had LIT tests that checked the IR and Dwarf and GDB execution and they failed in all levels more or less around December. This week I had time to finally refactor it and everything went back to normal (with lots of changes in the IR/Dwarf checks). To be honest, debugging metadata in IR is not trivial. You need to understand what's being built and how it gets used later, and for that you have to dig deep in the code, a problem that normal IR doesn't have. Not to mention the lack of validation on the parameters, metadata accepts anything you put in and the MDNode* behave almost like void*. But I have to say that the new DIBuilder is much closer to the metadata generated than the previous DIFactory, so maybe that'll get there in time... Great work Devang! It's really getting a lot nicer to build debug information. cheers, --renato
On Feb 25, 2011, at 5:23 AM, Renato Golin wrote:> On 24 February 2011 21:34, Jason Kim <jasonwkim at google.com> wrote: >> On Thu, Feb 24, 2011 at 1:29 PM, Devang Patel <dpatel at apple.com> wrote: > > It simplified a bit type construction but the overall code did not > reduce that much.The old interface expected FEs to keep track of everything, where as new interface tries to encapsulate as much info as possible. This should help cleanup FE code responsible to generate debugging information. I made a first pass in clang and simplified code little bit. There is more room for improvement now. If you look at CGDebugInfo.cpp in clang, you'll see "Unit" variable/argument used all over the place. IMO, majority of these could be eliminated thanks to DIBuilder. Another significant improvement is that it does not expose internal data structures (e.g. DerivedType, CompositeType... ) to FEs. The front-end should not be forced to learn about them and they do not directly represent any DWARF concept anyway. It is just an approach used my predecessor to communicate debug info from FE to llvm code generator. Now, with the encapsulation provided by DIBuilder, we have more freedom. - Devang
On 25 February 2011 16:55, Devang Patel <dpatel at apple.com> wrote:> The old interface expected FEs to keep track of everything, where as new interface tries to encapsulate as much info as possible. This should help cleanup FE code responsible to generate debugging information. I made a first pass in clang and simplified code little bit. There is more room for improvement now. If you look at CGDebugInfo.cpp in clang, you'll see "Unit" variable/argument used all over the place. IMO, majority of these could be eliminated thanks to DIBuilder.Yes, I got rid of that, too.> Another significant improvement is that it does not expose internal data structures (e.g. DerivedType, CompositeType... ) to FEs. The front-end should not be forced to learn about them and they do not directly represent any DWARF concept anyway. It is just an approach used my predecessor to communicate debug info from FE to llvm code generator.Indeed, I liked the way it's hidden in DIBuilder. Also the way typerefs are dealt with, too, is very good. Would be good to split CreateLocalVariable into CreateArgValue and CreateAutoVariable. Also would be good to have a simpler array types creation (CreateArrayType(size, align, ty, num_elms)). cheers, --renato