On Sat, Dec 22, 2012 at 2:21 AM, Vane, Edwin <[email protected]> wrote:
> I’m not expecting any support and I just want to make progress at this > point so I’ll make the changes and update the design doc to not mention > about llvm::Pass-like registration.**** > > ** ** > > Since not much of this patch is likely to remain, the first patch may just > get abandoned in favour of something more like the in-progress patch I > provided. > Thanks. Looking forward to reviewing it. > **** > > **** > > *From:* Manuel Klimek [mailto:[email protected]] > *Sent:* Friday, December 21, 2012 3:58 PM > > *To:* Vane, Edwin > *Cc:* [email protected]; > [email protected] > *Subject:* Re: [cfe-commits] [PATCH] Foundation for Transform > registration infrastructure**** > > ** ** > > Vane, I looked at the in-progress change - I like where this is going, but > I still think that having 1 place in main where you have to add the new > transform leads to significantly simpler to follow code (no magic!).**** > > ** ** > > If you find more people who would find the completely decoupling better, > I'm still happy to be outvoted.**** > > ** ** > > Cheers,**** > > /Manuel**** > > ** ** > > On Fri, Dec 21, 2012 at 7:11 PM, Vane, Edwin <[email protected]> wrote: > **** > > To help clear up the confusion, I’ve posted some preliminary work on > phabricator:**** > > **** > > http://llvm-reviews.chandlerc.com/D235**** > > **** > > This is my WIP porting loop-convert into cpp11-migrate. It still requires > polish but the things to note are the makefiles, what the Transform class > looks like, and how adding a new Transform is pretty much independent from > any other code with the exception of some changes to the build system. I’ll > submit a proper review for this code later. The purpose of this patch is to > help get the initial patch through.**** > > **** > > *From:* [email protected] [mailto: > [email protected]] *On Behalf Of *Vane, Edwin > *Sent:* Friday, December 21, 2012 9:22 AM > *To:* Manuel Klimek**** > > > *Cc:* [email protected]; > [email protected] > *Subject:* Re: [cfe-commits] [PATCH] Foundation for Transform > registration infrastructure**** > > **** > > There’s some confusion I’ll try to clear up here. I don’t want to use > Tablegen. It was just something Sean said about TableGenBackends.h where I > thought he was suggesting I use TableGen. After looking into the header I’m > pretty sure he didn’t actually mean that.**** > > **** > > I really do want a simple design with subclasses of Transform implementing > specific transformations and those subclasses being instantiated by main > based on what args are on the command-line. However, to satisfy the design > requirement of making transforms easy to add and not having to change any > central code every time a transform is added, I wanted to use something > like LLVM’s Pass mechanism for registration so the the pairing between > command-line arg and class to instantiate is contained within transform > code. This way a developer only has to add code to the tool and not have to > change anything in the existing code when adding a new transform.**** > > **** > > The registration mechanism as it exists on phabricator now works for this > purpose except that all sources need to be linked into the tool directly > and not via static libs or else the global registration variables don’t get > included. I have a solution in cmake which is very straightforward but the > legacy Makefile system is giving me issues. I’m debugging it now.**** > > **** > > *From:* Manuel Klimek [mailto:[email protected]] > *Sent:* Thursday, December 20, 2012 11:06 AM > *To:* Vane, Edwin > *Cc:* [email protected]; > [email protected] > *Subject:* Re: [cfe-commits] [PATCH] Foundation for Transform > registration infrastructure**** > > **** > > On Thu, Dec 20, 2012 at 3:39 PM, Vane, Edwin <[email protected]> wrote: > **** > > I wasn't planning on dynamically loading transforms. Yesterday when I > tried turning a transform into a static library to link into cpp11-migrate > I was reminded that unused global variables like the RegisterTransform<...> > vars won't get included in the link. My next goal is just to set up the > build system to slurp through subdirectories adding all sources to the > executable as one monolithic entity. I'm not sure how this is any less > complex than using tablegen although I admit I don't have much experience > with it.**** > > **** > > Why tablegen? I'm still not sure why we need anything but simple classes > that can be instantiated...**** > > Can you give examples of what you want to do that this wouldn't fulfill?** > ** > > **** > > Thanks,**** > > /Manuel**** > > **** > > > -----Original Message----- > From: Manuel Klimek [mailto:[email protected]]**** > > Sent: Thursday, December 20, 2012 2:43 AM > To: [email protected]; Vane, Edwin**** > > Cc: [email protected]; [email protected]; [email protected] > Subject: Re: [PATCH] Foundation for Transform registration infrastructure* > *** > > Sean's main point is my primary question: why do we need to load stuff > dynamically? > > I would expect to have a Transform class at some point, and have main > basically look like: > if (Flag1) > Transforms.push_back(new T1); > ... > and then do something with the vector of transforms we've built - that > wouldn't need any plugin infrastructure. > > PS: regarding my rants - many people in llvm disagree with my positions > here :) There's a lot of taste and personal preference to them, so if you > agree with my reasoning, cool, but always feel free to argue if you don't > agree :) I'm happy to learn new points of view > > http://llvm-reviews.chandlerc.com/D221**** > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits**** > > **** > > ** ** >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
