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> 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/20180326/8b74b963/attachment.html>
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/14ed72fa/attachment.html>
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>