From: Chandler Carruth [mailto:[email protected]] Sent: Thursday, October 04, 2012 11:46 AM To: Villmow, Micah Cc: Chris Lattner; [email protected] LLVM; [email protected] cfe Subject: Re: [llvm-commits] [cfe-commits] [Patch] Move TargetData from Target to Support/VMCore
On Thu, Oct 4, 2012 at 11:40 AM, Villmow, Micah <[email protected]<mailto:[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. =/ [Villmow, Micah] The problem is submitting patches for all the clients in a single commit. I would prefer to do each client separately so that if one breaks, I don't need to back out all the changes for every client. 3) Remove TargetData. > -----Original Message----- > From: Chris Lattner [mailto:[email protected]<mailto:[email protected]>] > Sent: Tuesday, October 02, 2012 10:02 PM > To: Villmow, Micah > Cc: Evan Cheng; Hal Finkel; > [email protected]<mailto:[email protected]> LLVM; Nadav Rotem; > [email protected]<mailto:[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]<mailto:[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]<mailto:[email protected]>] > >> Sent: Tuesday, October 02, 2012 9:26 AM > >> To: Villmow, Micah > >> Cc: Evan Cheng; Hal Finkel; > >> [email protected]<mailto:[email protected]> LLVM; Nadav > >> Rotem; [email protected]<mailto:[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]<mailto:[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]<mailto:[email protected]>] > >>>> Sent: Sunday, September 30, 2012 9:03 AM > >>>> To: Evan Cheng > >>>> Cc: Hal Finkel; Villmow, Micah; > >>>> [email protected]<mailto:[email protected]> LLVM; > >>>> Nadav Rotem; [email protected]<mailto:[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]<mailto:[email protected]>> > wrote: > >>>>> On Sep 26, 2012, at 11:07 AM, Hal Finkel > >>>>> <[email protected]<mailto:[email protected]>> wrote: > >>>>> > >>>>>> On Tue, 25 Sep 2012 16:16:22 -0700 Evan Cheng > >>>>>> <[email protected]<mailto:[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]<mailto:[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
