Bas van den Berg
2014-Jul-10 07:08 UTC
[LLVMdev] [lld] LLD's software architecture (update)
I've run a new scan of LLD's architecture and it appears to be getting cleaner! There are still some upward dependencies however: - core depends on Reader and Writer - passes depend on Reader and Writer - ReaderWriter/PECOFF/ReaderCOFF.cpp depends on Driver.h The updated architecture can be seen here: http://www.c2lang.org/docs/lld_architecture_20140710.png On Tue, Jun 3, 2014 at 3:07 AM, Rui Ueyama <ruiu at google.com> wrote:> I will create a patch to move the files and send it for review. > > > On Mon, Jun 2, 2014 at 1:05 PM, Michael Spencer <bigcheesegs at gmail.com> > wrote: > >> On Mon, Jun 2, 2014 at 12:49 AM, Rui Ueyama <ruiu at google.com> wrote: >> > I agree to move these files to Core. Any objections? >> >> None here. >> >> - Michael Spencer >> >> > >> > On Mon, Jun 2, 2014 at 12:44 AM, Bas van den Berg >> > <b.van.den.berg.nl at gmail.com> wrote: >> >> >> >> 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 >> >>>> >> >>> >> >> >> > >> > >> > _______________________________________________ >> > 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/20140710/8940827b/attachment.html>
Andreas Wendleder
2014-Jul-10 10:08 UTC
[LLVMdev] [lld] LLD's software architecture (update)
Hi - passes depend on Reader and Writer>That's an easy one. Compile tested on Windows with Cmake. Regards, Andreas>From 36855377f20b0492735339f36ed7c650a4c90624 Mon Sep 17 00:00:00 2001From: Andreas Wendleder <andreas.wendleder at gmail.com> Date: Thu, 10 Jul 2014 10:58:32 +0100 Subject: [PATCH] Remove LayoutPass dependency on ReaderWriter. It's not used anyway. --- include/lld/Passes/LayoutPass.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/lld/Passes/LayoutPass.h b/include/lld/Passes/LayoutPass.h index 3392480..ffb633d 100644 --- a/include/lld/Passes/LayoutPass.h +++ b/include/lld/Passes/LayoutPass.h @@ -12,7 +12,6 @@ #include "lld/Core/File.h" #include "lld/Core/Pass.h" -#include "lld/ReaderWriter/Reader.h" #include "llvm/ADT/DenseMap.h" -- 1.9.2.msysgit.0 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140710/b2a8bb5b/attachment.html> -------------- next part -------------- A non-text attachment was scrubbed... Name: 0001-Remove-LayoutPass-dependency-on-ReaderWriter.patch Type: application/octet-stream Size: 680 bytes Desc: not available URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140710/b2a8bb5b/attachment.obj>
Bas van den Berg
2014-Jul-10 11:10 UTC
[LLVMdev] [lld] LLD's software architecture (update)
Well, it does have a Registry& that's defined in Reader.h. But that can be forward declared (and apparently is in some of the other headers it includes. On Thu, Jul 10, 2014 at 12:08 PM, Andreas Wendleder < andreas.wendleder at googlemail.com> wrote:> Hi > > - passes depend on Reader and Writer >> > > That's an easy one. Compile tested on Windows with Cmake. > > Regards, > Andreas > > From 36855377f20b0492735339f36ed7c650a4c90624 Mon Sep 17 00:00:00 2001 > From: Andreas Wendleder <andreas.wendleder at gmail.com> > Date: Thu, 10 Jul 2014 10:58:32 +0100 > Subject: [PATCH] Remove LayoutPass dependency on ReaderWriter. > > It's not used anyway. > --- > include/lld/Passes/LayoutPass.h | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/include/lld/Passes/LayoutPass.h > b/include/lld/Passes/LayoutPass.h > index 3392480..ffb633d 100644 > --- a/include/lld/Passes/LayoutPass.h > +++ b/include/lld/Passes/LayoutPass.h > @@ -12,7 +12,6 @@ > > #include "lld/Core/File.h" > #include "lld/Core/Pass.h" > -#include "lld/ReaderWriter/Reader.h" > > #include "llvm/ADT/DenseMap.h" > > -- > 1.9.2.msysgit.0 > > > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140710/d957aed8/attachment.html>