It seems a little strange conceptually that object depends on BitcodeReader. Is it possible to break that dependency? On Thu, Mar 29, 2018 at 4:11 PM David Blaikie <dblaikie at gmail.com> wrote:> On Mon, Mar 26, 2018 at 4:52 PM David Blaikie <dblaikie at gmail.com> wrote: > >> On Thu, Mar 22, 2018 at 12:55 PM Reid Kleckner <rnk at google.com> wrote: >> >>> On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com> >>> wrote: >>> >>>> I'm looking at fixing some layering violations in LLVM & came across a >>>> few in the CodeView handling, specifically: >>>> >>>> lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers >>>> I guess MC could be made dependent on DebugInfoCodeView? But probably >>>> these things should be sunk into BinaryFormat as is the case for DWARF >>>> features used by MC? >>>> >>> >>> I'd be OK with that. We could very easily introduce true link >>> dependencies on that library in the near future. >>> >> >> OK - added the LLVMBuild.txt change in r328595 >> > > Hmm, not perfect - this creates a bit of a circular dependency: > > llvm-tblgen > MC > DebugInfo/CodeView > Object > Bitcode/Reader > IR (including lib/IR/AttributesCompatFunc.inc) > llvm-tblgen (to generate AttributesCompatFunc.inc) > > This is just a specific path shown by the internal build system - no doubt > there are other tblgen files in LLVM IR, etc. > > So I guess still need to split up DebugInfo/CodeView (assuming the bits > needed by MC don't depend on Object)? Any suggestions? Sink it into > BinaryFormat or name a new library? > > (it's quite a few headers by the looks of it: CodeView.h, Line.h, > SymbolRecord.h, CodeViewTypes.def, CodeViewSymbols.def, > CodeViewRegisters.def, TypeIndex.h, CVRecord.h, CodeViewError.h, > RecordSerialization.h - maybe some of those aren't needed, if they're > excessive includes or the like (they tend to seep in all over the place)) > > >> >> >>> include/llvm/Object/COFF.h includes >>>> include/llvm/DebugInfo/CodeView/CVDebugRecord.h >>>> Also seems like this could/should/needs to be sunk into BinaryFormat? >>>> >>> >>> These things are really more PE/PDB related than CodeView. They are in >>> the wrong place. I'd ask Saleem to help find a new home. Object or >>> BinaryFormats seem OK to me. >>> >> >> Moved it to libObject (r328593) for now which is at least correct - if >> someone thinks it'd be more suited to BinaryFormat, I'm happy to move it or >> they can. >> >> >>> >>> >>>> I'm open to ideas & happy to do the work, or help in any way that might >>>> be useful. >>>> >>>> Thanks, >>>> - Dave >>>> >>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180329/4f2cbe32/attachment.html>
No, Object is supposed to be an abstraction over real object files and LLVM bitcode object files. Maybe we can break the CodeView -> Object dependency. On Thu, Mar 29, 2018 at 4:23 PM Zachary Turner <zturner at google.com> wrote:> It seems a little strange conceptually that object depends on > BitcodeReader. Is it possible to break that dependency? > On Thu, Mar 29, 2018 at 4:11 PM David Blaikie <dblaikie at gmail.com> wrote: > >> On Mon, Mar 26, 2018 at 4:52 PM David Blaikie <dblaikie at gmail.com> wrote: >> >>> On Thu, Mar 22, 2018 at 12:55 PM Reid Kleckner <rnk at google.com> wrote: >>> >>>> On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com> >>>> wrote: >>>> >>>>> I'm looking at fixing some layering violations in LLVM & came across a >>>>> few in the CodeView handling, specifically: >>>>> >>>>> lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers >>>>> I guess MC could be made dependent on DebugInfoCodeView? But >>>>> probably these things should be sunk into BinaryFormat as is the case for >>>>> DWARF features used by MC? >>>>> >>>> >>>> I'd be OK with that. We could very easily introduce true link >>>> dependencies on that library in the near future. >>>> >>> >>> OK - added the LLVMBuild.txt change in r328595 >>> >> >> Hmm, not perfect - this creates a bit of a circular dependency: >> >> llvm-tblgen >> MC >> DebugInfo/CodeView >> Object >> Bitcode/Reader >> IR (including lib/IR/AttributesCompatFunc.inc) >> llvm-tblgen (to generate AttributesCompatFunc.inc) >> >> This is just a specific path shown by the internal build system - no >> doubt there are other tblgen files in LLVM IR, etc. >> >> So I guess still need to split up DebugInfo/CodeView (assuming the bits >> needed by MC don't depend on Object)? Any suggestions? Sink it into >> BinaryFormat or name a new library? >> >> (it's quite a few headers by the looks of it: CodeView.h, Line.h, >> SymbolRecord.h, CodeViewTypes.def, CodeViewSymbols.def, >> CodeViewRegisters.def, TypeIndex.h, CVRecord.h, CodeViewError.h, >> RecordSerialization.h - maybe some of those aren't needed, if they're >> excessive includes or the like (they tend to seep in all over the place)) >> >> >>> >>> >>>> include/llvm/Object/COFF.h includes >>>>> include/llvm/DebugInfo/CodeView/CVDebugRecord.h >>>>> Also seems like this could/should/needs to be sunk into BinaryFormat? >>>>> >>>> >>>> These things are really more PE/PDB related than CodeView. They are in >>>> the wrong place. I'd ask Saleem to help find a new home. Object or >>>> BinaryFormats seem OK to me. >>>> >>> >>> Moved it to libObject (r328593) for now which is at least correct - if >>> someone thinks it'd be more suited to BinaryFormat, I'm happy to move it or >>> they can. >>> >>> >>>> >>>> >>>>> I'm open to ideas & happy to do the work, or help in any way that >>>>> might be useful. >>>>> >>>>> Thanks, >>>>> - Dave >>>>> >>>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180330/8f5147a5/attachment.html>
Looks like maybe the CodeView -> Object dependency is out of date/old/not needed any more anyway... (don't see any Object headers included from the CodeView headers or implementation, etc). Will see if going that way internally is viable & loop back if it stumbles across something. On Thu, Mar 29, 2018 at 5:27 PM Reid Kleckner <rnk at google.com> wrote:> No, Object is supposed to be an abstraction over real object files and > LLVM bitcode object files. > > Maybe we can break the CodeView -> Object dependency. > > On Thu, Mar 29, 2018 at 4:23 PM Zachary Turner <zturner at google.com> wrote: > >> It seems a little strange conceptually that object depends on >> BitcodeReader. Is it possible to break that dependency? >> On Thu, Mar 29, 2018 at 4:11 PM David Blaikie <dblaikie at gmail.com> wrote: >> >>> On Mon, Mar 26, 2018 at 4:52 PM David Blaikie <dblaikie at gmail.com> >>> wrote: >>> >>>> On Thu, Mar 22, 2018 at 12:55 PM Reid Kleckner <rnk at google.com> wrote: >>>> >>>>> On Wed, Mar 21, 2018 at 11:31 AM David Blaikie <dblaikie at gmail.com> >>>>> wrote: >>>>> >>>>>> I'm looking at fixing some layering violations in LLVM & came across >>>>>> a few in the CodeView handling, specifically: >>>>>> >>>>>> lib/MC/MCCodeView includes several llvm/DebugInfo/CodeView headers >>>>>> I guess MC could be made dependent on DebugInfoCodeView? But >>>>>> probably these things should be sunk into BinaryFormat as is the case for >>>>>> DWARF features used by MC? >>>>>> >>>>> >>>>> I'd be OK with that. We could very easily introduce true link >>>>> dependencies on that library in the near future. >>>>> >>>> >>>> OK - added the LLVMBuild.txt change in r328595 >>>> >>> >>> Hmm, not perfect - this creates a bit of a circular dependency: >>> >>> llvm-tblgen >>> MC >>> DebugInfo/CodeView >>> Object >>> Bitcode/Reader >>> IR (including lib/IR/AttributesCompatFunc.inc) >>> llvm-tblgen (to generate AttributesCompatFunc.inc) >>> >>> This is just a specific path shown by the internal build system - no >>> doubt there are other tblgen files in LLVM IR, etc. >>> >>> So I guess still need to split up DebugInfo/CodeView (assuming the bits >>> needed by MC don't depend on Object)? Any suggestions? Sink it into >>> BinaryFormat or name a new library? >>> >>> (it's quite a few headers by the looks of it: CodeView.h, Line.h, >>> SymbolRecord.h, CodeViewTypes.def, CodeViewSymbols.def, >>> CodeViewRegisters.def, TypeIndex.h, CVRecord.h, CodeViewError.h, >>> RecordSerialization.h - maybe some of those aren't needed, if they're >>> excessive includes or the like (they tend to seep in all over the place)) >>> >>> >>>> >>>> >>>>> include/llvm/Object/COFF.h includes >>>>>> include/llvm/DebugInfo/CodeView/CVDebugRecord.h >>>>>> Also seems like this could/should/needs to be sunk into >>>>>> BinaryFormat? >>>>>> >>>>> >>>>> These things are really more PE/PDB related than CodeView. They are in >>>>> the wrong place. I'd ask Saleem to help find a new home. Object or >>>>> BinaryFormats seem OK to me. >>>>> >>>> >>>> Moved it to libObject (r328593) for now which is at least correct - if >>>> someone thinks it'd be more suited to BinaryFormat, I'm happy to move it or >>>> they can. >>>> >>>> >>>>> >>>>> >>>>>> I'm open to ideas & happy to do the work, or help in any way that >>>>>> might be useful. >>>>>> >>>>>> Thanks, >>>>>> - Dave >>>>>> >>>>>-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180401/ac6705ae/attachment.html>