[cfe-commits] [PATCH] Foundation for Transform registration infrastructure

Sat Dec 22 02:24:51 PST 2012

On Sat, Dec 22, 2012 at 2:21 AM, Vane, Edwin <edwin.vane at intel.com> 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:klimek at google.com]
> *Sent:* Friday, December 21, 2012 3:58 PM
>
> *To:* Vane, Edwin
> *Cc:* reviews+D221+public+4c9537db733ade53 at llvm-reviews.chandlerc.com;
> cfe-commits at cs.uiuc.edu
> *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 <edwin.vane at intel.com> 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:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Vane, Edwin
> *Sent:* Friday, December 21, 2012 9:22 AM
> *To:* Manuel Klimek****
>
>
> *Cc:* reviews+D221+public+4c9537db733ade53 at llvm-reviews.chandlerc.com;
> cfe-commits at cs.uiuc.edu
> *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:klimek at google.com]
> *Sent:* Thursday, December 20, 2012 11:06 AM
> *To:* Vane, Edwin
> *Cc:* reviews+D221+public+4c9537db733ade53 at llvm-reviews.chandlerc.com;
> cfe-commits at cs.uiuc.edu
> *Subject:* Re: [cfe-commits] [PATCH] Foundation for Transform
> registration infrastructure****
>
>  ****
>
> On Thu, Dec 20, 2012 at 3:39 PM, Vane, Edwin <edwin.vane at intel.com> 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:klimek at google.com]****
>
> Sent: Thursday, December 20, 2012 2:43 AM
> To: klimek at google.com; Vane, Edwin****
>
> Cc: cfe-commits at cs.uiuc.edu; gribozavr at gmail.com; silvas at purdue.edu
> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits****
>
>   ****
>
> ** **
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121222/6e331731/attachment.html>