llvm dev - Jul 2014 - [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>

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/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>

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>