Bas van den Berg
2014-Jun-02 07:01 UTC
[LLVMdev] [lld] LLD's software architecture (update)
Hi, I've been following the changes in LLD's software architecture. Recently a new file was added: Alias.h In this file, the curret Atom set is extended with an AliasAtom. While the change seems innocent enough, it has some nasty potential. Simple.h and Alias.h are placed in the ReaderWriter component. This is the 2nd component in the layering: Driver ReaderWriter Passes Core The problem is that Core depends on these files. So in my opinion, they belong to that Component. The greater impact here is that once core starts depending on ReaderWriter, architecture Erosion will happen. This would obviously not be a good thing, since the current design is fresh and clean. I would love to supply patches, but am having issues getting lld to build under Ubuntu 12.04. (but that's another story ;) ). I've posted the updated architecture view here: http://www.c2lang.org/docs/lld_architecture_20140602.png -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/eaa5b5af/attachment.html>
On Mon, Jun 2, 2014 at 12:01 AM, Bas van den Berg < b.van.den.berg.nl at gmail.com> wrote:> Hi, > > I've been following the changes in LLD's software architecture. > Recently a new file was added: Alias.h > In this file, the curret Atom set is extended with an AliasAtom. > While the change seems innocent enough, it has some nasty > potential. Simple.h and Alias.h are placed in the ReaderWriter > component. This is the 2nd component in the layering: > Driver > ReaderWriter > Passes > Core > > The problem is that Core depends on these files. So in my opinion, > they belong to that Component. The greater impact here is that > once core starts depending on ReaderWriter, architecture Erosion > will happen. This would obviously not be a good thing, since the > current design is fresh and clean. >Core's dependency to Simple.h was there before the addition of Alias.h, so it's not new. And I don't feel like it's that bad as you might be feeling. Do you have any suggestion to get rid of it? Just moving the file to Core? I would love to supply patches, but am having issues getting> lld to build under Ubuntu 12.04. (but that's another story ;) ). > > I've posted the updated architecture view here: > http://www.c2lang.org/docs/lld_architecture_20140602.png > > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/11c9195c/attachment.html>
Bas van den Berg
2014-Jun-02 07:44 UTC
[LLVMdev] [lld] LLD's software architecture (update)
The inverted dependency of Core to ReaderWriter via Simple.h was already present. My idea was to fix the problem before it gets bigger. My proposal would be to move Simple.h and Alias.h to Core. Similar to UndefinedAtom.h etc. It would be even nicer to make the naming consistent as well, since there already is UndefinedAtom.h SharedLibaryAtom.h etc Maybe: SimpleAtom.h AliasAtom.h Moving the file to core fixes the dependencies. On Mon, Jun 2, 2014 at 9:25 AM, Rui Ueyama <ruiu at google.com> wrote:> On Mon, Jun 2, 2014 at 12:01 AM, Bas van den Berg < > b.van.den.berg.nl at gmail.com> wrote: > >> Hi, >> >> I've been following the changes in LLD's software architecture. >> Recently a new file was added: Alias.h >> In this file, the curret Atom set is extended with an AliasAtom. >> While the change seems innocent enough, it has some nasty >> potential. Simple.h and Alias.h are placed in the ReaderWriter >> component. This is the 2nd component in the layering: >> Driver >> ReaderWriter >> Passes >> Core >> >> The problem is that Core depends on these files. So in my opinion, >> they belong to that Component. The greater impact here is that >> once core starts depending on ReaderWriter, architecture Erosion >> will happen. This would obviously not be a good thing, since the >> current design is fresh and clean. >> > > Core's dependency to Simple.h was there before the addition of Alias.h, so > it's not new. And I don't feel like it's that bad as you might be feeling. > Do you have any suggestion to get rid of it? Just moving the file to Core? > > I would love to supply patches, but am having issues getting >> lld to build under Ubuntu 12.04. (but that's another story ;) ). >> >> I've posted the updated architecture view here: >> http://www.c2lang.org/docs/lld_architecture_20140602.png >> >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140602/a4a02fe7/attachment.html>