search for: ref_addr

Hi All, The first patch adds support for ref_addr. Most of it is from r176882, but instead of always using an integer for ref_addr, we use label + offset for relocation on non-darwin platforms. The second patch is a modified version of r191792. The main change is to use a single map instead of 3 maps in DwarfDebug and instead of calling DwarfDebu...

Ping -Manman On Fri, Oct 4, 2013 at 7:00 PM, Manman Ren <manman.ren at gmail.com> wrote: > > Hi All, > > The first patch adds support for ref_addr. > Most of it is from r176882, but instead of always using an integer for > ref_addr, we use label + offset for relocation on non-darwin platforms. > > The second patch is a modified version of r191792. > The main change is to use a single map instead of 3 maps in DwarfDebug and >...

...more test coverage while fixing this issue. Eric/Manman: rough design question: compute the absolute offset of each CU within the debug_info section and describe them all relative to a single symbol at the start of the debug_info section, or should we put a label at the start of each CU? 0002: ref_addr_relocation.ll: seems a bit vague in terms of how you test for the relocation. I think it'd make more sense to test the assembly, than the reafobj output, that way you can test that the correct bytes have the relocation rather than just that there's "some" .debug_info relocatio...

...t; Might be easier if these were on Phabricator, but here are some thoughts: > > 0001: > This patch generally, while separated for legibility, is untested & > difficult to discuss in isolation. > I agree, this patch adds the functionality but does not use it, the 2nd patch uses ref_addr. If you think I should merge the two and commit as a single patch, let me know. > I may need to refer to the second patch in reviewing this first one. > DwarfDebug.cpp: > computeSizeAndOffsets: > I believe this produces the wrong offset for the 3rd CU and onwards. > com...

...are some thoughts: >>> >>> 0001: >>> This patch generally, while separated for legibility, is untested & >>> difficult to discuss in isolation. >>> >> I agree, this patch adds the functionality but does not use it, the 2nd >> patch uses ref_addr. >> If you think I should merge the two and commit as a single patch, let me >> know. >> >> >>> I may need to refer to the second patch in reviewing this first one. >>> DwarfDebug.cpp: >>> computeSizeAndOffsets: >>> I believe...

The first patch seems fine, though the comment on the modified addDIEEntry function is a bit confusing: -/// addDIEEntry - Add a DIE attribute data and value. +/// addDIEEntry - Add a DIE attribute data and value. The form should be +/// a reference form: ref1, ref2, ref4, ref8, ref_udata, ref_addr, +/// or ref_sig8. A form can be chosen inside addDIEEntry. When the comment says "The form should be" - it sounds like it /could/ be something else, etc. As though the caller would specify it and must meet some requirement. But the caller doesn't specify it at all. I'd probably...

...re on Phabricator, but here are some thoughts: >> >> 0001: >> This patch generally, while separated for legibility, is untested & >> difficult to discuss in isolation. >> > I agree, this patch adds the functionality but does not use it, the 2nd > patch uses ref_addr. > If you think I should merge the two and commit as a single patch, let me > know. > > >> I may need to refer to the second patch in reviewing this first one. >> DwarfDebug.cpp: >> computeSizeAndOffsets: >> I believe this produces the wrong offset f...

...> 0001: >>>>> This patch generally, while separated for legibility, is untested & >>>>> difficult to discuss in isolation. >>>>> >>>> I agree, this patch adds the functionality but does not use it, the 2nd >>>> patch uses ref_addr. >>>> If you think I should merge the two and commit as a single patch, let >>>> me know. >>>> >>>> >>>>> I may need to refer to the second patch in reviewing this first one. >>>>> DwarfDebug.cpp: >>>>>...

...gt;> >>>> 0001: >>>> This patch generally, while separated for legibility, is untested & >>>> difficult to discuss in isolation. >>>> >>> I agree, this patch adds the functionality but does not use it, the 2nd >>> patch uses ref_addr. >>> If you think I should merge the two and commit as a single patch, let me >>> know. >>> >>> >>>> I may need to refer to the second patch in reviewing this first one. >>>> DwarfDebug.cpp: >>>> computeSizeAndOffsets: &...

...This patch generally, while separated for legibility, is untested >>>>>>> & difficult to discuss in isolation. >>>>>>> >>>>>> I agree, this patch adds the functionality but does not use it, the >>>>>> 2nd patch uses ref_addr. >>>>>> If you think I should merge the two and commit as a single patch, let >>>>>> me know. >>>>>> >>>>>> >>>>>>> I may need to refer to the second patch in reviewing this first one. >>>>>...

...generally, while separated for legibility, is untested >>>>>>>> & difficult to discuss in isolation. >>>>>>> >>>>>>> I agree, this patch adds the functionality but does not use it, the >>>>>>> 2nd patch uses ref_addr. >>>>>>> If you think I should merge the two and commit as a single patch, let >>>>>>> me know. >>>>>>> >>>>>>>> >>>>>>>> I may need to refer to the second patch in reviewing this first on...

...are some thoughts: >>> >>> 0001: >>> This patch generally, while separated for legibility, is untested & >>> difficult to discuss in isolation. >>> >> I agree, this patch adds the functionality but does not use it, the 2nd >> patch uses ref_addr. >> If you think I should merge the two and commit as a single patch, let me >> know. >> >> >>> I may need to refer to the second patch in reviewing this first one. >>> DwarfDebug.cpp: >>> computeSizeAndOffsets: >>> I believe...

...gt;>>> This patch generally, while separated for legibility, is untested & >>>>>> difficult to discuss in isolation. >>>>>> >>>>> I agree, this patch adds the functionality but does not use it, the >>>>> 2nd patch uses ref_addr. >>>>> If you think I should merge the two and commit as a single patch, let >>>>> me know. >>>>> >>>>> >>>>>> I may need to refer to the second patch in reviewing this first one. >>>>>> DwarfDebug.cp...

...This patch generally, while separated for legibility, is untested >>>>>>> & difficult to discuss in isolation. >>>>>>> >>>>>> I agree, this patch adds the functionality but does not use it, the >>>>>> 2nd patch uses ref_addr. >>>>>> If you think I should merge the two and commit as a single patch, let >>>>>> me know. >>>>>> >>>>>> >>>>>>> I may need to refer to the second patch in reviewing this first one. >>>>>...

On Wed, Oct 16, 2013 at 1:33 PM, Manman Ren <manman.ren at gmail.com> wrote: > >> patch looks fairly obvious/trivial >> >> Have you tried any of the test cases I've described (special members, >> nested types, and member templates - all used across CUs so an earlier CU >> has the type definition and a latter one has one of these extra members)? >>

...parated for legibility, is >>>>>>>>> untested & difficult to discuss in isolation. >>>>>>>>> >>>>>>>> I agree, this patch adds the functionality but does not use it, the >>>>>>>> 2nd patch uses ref_addr. >>>>>>>> If you think I should merge the two and commit as a single patch, >>>>>>>> let me know. >>>>>>>> >>>>>>>> >>>>>>>>> I may need to refer to the second patch in review...

...erally, while separated for legibility, is untested >>>>>>>> & difficult to discuss in isolation. >>>>>>>> >>>>>>> I agree, this patch adds the functionality but does not use it, the >>>>>>> 2nd patch uses ref_addr. >>>>>>> If you think I should merge the two and commit as a single patch, >>>>>>> let me know. >>>>>>> >>>>>>> >>>>>>>> I may need to refer to the second patch in reviewing this first >&...

> > > patch looks fairly obvious/trivial > > Have you tried any of the test cases I've described (special members, > nested types, and member templates - all used across CUs so an earlier CU > has the type definition and a latter one has one of these extra members)? > I'd like to see test results for these before we progress. > Yes, templates and nested types

On Wed, Oct 16, 2013 at 2:00 PM, David Blaikie <dblaikie at gmail.com> wrote: > > > > On Wed, Oct 16, 2013 at 1:33 PM, Manman Ren <manman.ren at gmail.com> wrote: > >> >>> patch looks fairly obvious/trivial >>> >>> Have you tried any of the test cases I've described (special members, >>> nested types, and member templates -

...gt;>>>>>>>>> untested & difficult to discuss in isolation. >>>>>>>>>>> >>>>>>>>>> I agree, this patch adds the functionality but does not use it, >>>>>>>>>> the 2nd patch uses ref_addr. >>>>>>>>>> If you think I should merge the two and commit as a single patch, >>>>>>>>>> let me know. >>>>>>>>>> >>>>>>>>>> >>>>>>>>>>> I may nee...