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

[Villmow, Micah]

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).
2) Update each client to point to DataLayout.
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>
>