[cfe-commits] [llvm-commits] [Patch] Move TargetData from Target to Support/VMCore

Chandler Carruth chandlerc at google.com
Thu Oct 4 11:45:42 PDT 2012


On Thu, Oct 4, 2012 at 11:40 AM, Villmow, Micah <Micah.Villmow at amd.com>wrote:

> Chris, the problem with steps #2/#3 is that plenty of clients have forward
> declarations of TargetData and the typedef won't work in this case, so I
> need to update the clients anyways.
>
> What about this sequence:
> 1) Introduce include/llvm/DataLayout.h and lib/VMCore/DataLayout.cpp(which
> is a functionally equivalent copy of TargetData).
>

First just copy the files over without any edits, then in a second commit
rename stuff to DataLayout. That way we can see the rename. It's all dead
code until #2


> 2) Update each client to point to DataLayout.
>

This step needs to be a single commit.

Otherwise, this seems a reasonable way to cope with forward declarations.
They make renaming really hard. =/


> 3) Remove TargetData.
>



>
>
> > -----Original Message-----
> > From: Chris Lattner [mailto:clattner at apple.com]
> > Sent: Tuesday, October 02, 2012 10:02 PM
> > To: Villmow, Micah
> > Cc: Evan Cheng; Hal Finkel; llvm-commits at cs.uiuc.edu LLVM; Nadav Rotem;
> > cfe-commits at cs.uiuc.edu cfe
> > Subject: Re: [cfe-commits] [llvm-commits] [Patch] Move TargetData from
> > Target to Support/VMCore
> >
> >
> > On Oct 2, 2012, at 5:57 PM, "Villmow, Micah" <Micah.Villmow at amd.com>
> > wrote:
> >
> > > Chris,
> > > Here is an attachd patch. It is quite large because the number of
> > places within the LLVM tree where TargetData is used.
> > >
> > > My only question is did you want a new subdirectory, VMCore, created
> > in the include directory?
> >
> > My mistake, sorry for being unclear.  More specifically, please do this
> > as a series of independent patches:
> >
> > 1. Add a typedef for TargetData -> DataLayout in TargetData.h 2. Rename
> > the TargetData class & implementation (but not the file or clients) to
> > DataLayout.  Switch the typedef to DataLayout -> TargetData.
> > 3. Rename uses of the "TargetData" typedef to be DataLayout.  Remove the
> > typedef.
> > 4. Move lib/Target/TargetData.cpp to lib/VMCore/DataLayout.cpp 5. Move
> > the contents the header from include/llvm/Target/TargetData.h to
> > include/llvm/DataLayout.h and keep TargetData.h as just a single
> > #include of the new header.
> > 6. Update all the clients to switch from TargetData.h to DataLayout.h in
> > the various projects (clang/llvm/dragonegg/lldb) that include it.  When
> > they're all converted, remove the forwarding header.
> >
> > The idea of each of these steps is that they become "obvious" and really
> > simple to review.  Thanks for tackling this Micah!
> >
> > -Chris
> >
> > >
> > > Micah
> > >
> > >> -----Original Message-----
> > >> From: Chris Lattner [mailto:clattner at apple.com]
> > >> Sent: Tuesday, October 02, 2012 9:26 AM
> > >> To: Villmow, Micah
> > >> Cc: Evan Cheng; Hal Finkel; llvm-commits at cs.uiuc.edu LLVM; Nadav
> > >> Rotem; cfe-commits at cs.uiuc.edu cfe
> > >> Subject: Re: [cfe-commits] [llvm-commits] [Patch] Move TargetData
> > >> from Target to Support/VMCore
> > >>
> > >> Yes.  How about just llvm/VMCore/DataLayout.h?
> > >>
> > >> -Chris
> > >>
> > >> On Oct 2, 2012, at 8:55 AM, "Villmow, Micah" <Micah.Villmow at amd.com>
> > >> wrote:
> > >>
> > >>> Chris,
> > >>> So if I renamed it to something like DataLayoutParser, that would be
> > >> acceptable to move the functionality into core?
> > >>>
> > >>> Micah
> > >>>
> > >>>> -----Original Message-----
> > >>>> From: Chris Lattner [mailto:clattner at apple.com]
> > >>>> Sent: Sunday, September 30, 2012 9:03 AM
> > >>>> To: Evan Cheng
> > >>>> Cc: Hal Finkel; Villmow, Micah; llvm-commits at cs.uiuc.edu LLVM;
> > >>>> Nadav Rotem; cfe-commits at cs.uiuc.edu cfe
> > >>>> Subject: Re: [cfe-commits] [llvm-commits] [Patch] Move TargetData
> > >>>> from Target to Support/VMCore
> > >>>>
> > >>>> On Sep 26, 2012, at 9:18 PM, Evan Cheng <evan.cheng at apple.com>
> > wrote:
> > >>>>> On Sep 26, 2012, at 11:07 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> > >>>>>
> > >>>>>> On Tue, 25 Sep 2012 16:16:22 -0700 Evan Cheng
> > >>>>>> <evan.cheng at apple.com> wrote:
> > >>>>>>
> > >>>>>>> Sorry, I understand why you are requesting this but I thinking
> > >>>>>>> moving TargetData to support is conceptually dirty.
> > >>>>>>
> > >>>>>> Can you please explain this? I think that the opposite is true:
> > >>>>>> Having TargetData in Target is conceptually dirty. TargetData
> > >>>>>> represents 'target information that is available to frontends and
> > >>>>>> IR-level passes without linking to the target descriptions'.
> > >>>>>
> > >>>>> Agreed.
> > >>>>>
> > >>>>>> As a result, I feel
> > >>>>>> that TargetData does not belong with the target-description
> > >>>>>> infrastructure, and so it should be moved out of Target so that
> > >>>>>> everyone can use it.
> > >>>>>
> > >>>>> I agree it should be moved out but at least it's target related.
> > >>>> Polluting Support / VMCore with it is just worse. They have nothing
> > >>>> to do with target data conceptually. This is all a matter of taste.
> > >>>> I'll let Chris make the decision.
> > >>>>
> > >>>> I agree with this patch in principle: TargetData should be moved to
> > >>>> VMCore.  However, the class should also be renamed.
> > >>>>
> > >>>> -Chris
> > >>>
> > >>>
> > >
> > > <move_TargetData_to_DataLayout.txt>
> >
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121004/e417e122/attachment.html>


More information about the cfe-commits mailing list