llvm dev - Jul 2014 - [LLVMdev] [lld] LLD's software architecture (update)

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
>

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/20140602/f7d7f49c/attachment.html>

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>