On Thu, Oct 4, 2012 at 11:40 AM, Villmow, Micah <[email protected]>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:[email protected]] > > Sent: Tuesday, October 02, 2012 10:02 PM > > To: Villmow, Micah > > Cc: Evan Cheng; Hal Finkel; [email protected] LLVM; Nadav Rotem; > > [email protected] 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" <[email protected]> > > 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:[email protected]] > > >> Sent: Tuesday, October 02, 2012 9:26 AM > > >> To: Villmow, Micah > > >> Cc: Evan Cheng; Hal Finkel; [email protected] LLVM; Nadav > > >> Rotem; [email protected] 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" <[email protected]> > > >> 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:[email protected]] > > >>>> Sent: Sunday, September 30, 2012 9:03 AM > > >>>> To: Evan Cheng > > >>>> Cc: Hal Finkel; Villmow, Micah; [email protected] LLVM; > > >>>> Nadav Rotem; [email protected] 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 <[email protected]> > > wrote: > > >>>>> On Sep 26, 2012, at 11:07 AM, Hal Finkel <[email protected]> wrote: > > >>>>> > > >>>>>> On Tue, 25 Sep 2012 16:16:22 -0700 Evan Cheng > > >>>>>> <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
