[PATCH] Move CodeGenPrepare into lib/CodeGen

[Quentin Colombet]

On Feb 21, 2014, at 3:29 PM, Chandler Carruth <chandlerc at google.com> wrote:

> I'm moderately concerned that no one will actually follow through on this, but I suppose at the worst I will.
Aren’t you writing a RFC?
This will draw the attention of the community and hopefully, several people would be interested in helping here.
I also think there is something to clean up here, but I will not commit on doing that as this is not a promise I am sure to honor.

> Since this fixes a build failure for some folks, yea, go for it.
Thanks, committed as r201912.

> 
> +  // For codegen passes, only passes that does IR to IR transformation are
> +  // supported. For now, just add CodeGenPrepare.
> +  initializeCodeGenPreparePass(Registry);
> 
> s/does/do/
Good catch!

> 
> This bit also highlights that there is a *lot* more work to do to make this a reasonable model. =/ We've sunk the hacks into opt.cpp essentially, which is better, but still a bit hacky.
> 
I agree. Now that I have been taught about the intended design (thanks to you, Hal, and Lang), I also fell this is something we have to fix eventually.

Thanks,
-Quentin
> 
> On Fri, Feb 21, 2014 at 1:32 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Chandler,
> 
> I know you are working on a RFC for the more general problem of layer violation. In the meantime, could I commit this innocent patch? (Do not be suspicious I said it is innocent :)).
> 
> Thanks,
> -Quentin
> 
> On Feb 20, 2014, at 1:40 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Hi Lang,
>> 
>> Thanks for the update, I better understand the underlying problem now :).
>> 
>> On Feb 20, 2014, at 11:09 AM, Lang Hames <lhames at gmail.com> wrote:
>> 
>>> Hi All,
>>> 
>>> Summarizing some discussion about this on IRC: Hal is right, opt
>>> already depends on CodeGen (indirectly via a dependence on the
>>> targets). So I renounce my concern on that point - having opt run CGP
>>> isn't a problem, at least in terms of changing dependencies. As for
>>> bugpoint, it does not currently depend on CodeGen, so that really is a
>>> new dependence. I'm not aware of any problems with that though.
>>> 
>>>> As one can notice (makefile, CMakeLists) the patch introduces a dependency
>>>> on libLLVMCodeGen for both opt and bugpoint because of that.
>>> 
>>> Quentin - out of interest, do you actually need to specify the
>>> dependence on CodeGen for opt. From what we discovered it seems like
>>> using CGP from opt should just work?
>> No, we don’t but I thought it was cleaner that way.
>> 
>>> 
>>> I think Chandler's point is still valid - if opt and llc depend on the
>>> same libraries it's not clear that there's any value to making them
>>> separate tools.
>> I just saw some part of the discuss on IRC. It is clearer now what was the concern and indeed, I was not planing on addressing that here :).
>> 
>> Thanks,
>> -Quentin
>> 
>>> That issue is only loosely related to this patch
>>> though, and shouldn't stop it going in.
>>> 
>>> Cheers,
>>> Lang.
>>> 
>>> On Thu, Feb 20, 2014 at 10:20 AM, Hal Finkel <hfinkel at anl.gov> wrote:
>>>> ----- Original Message -----
>>>>> From: "Lang Hames" <lhames at gmail.com>
>>>>> To: "Tom Stellard" <tom at stellard.net>
>>>>> Cc: "Hal Finkel" <hfinkel at anl.gov>, "Commit Messages and Patches for LLVM" <llvm-commits at cs.uiuc.edu>
>>>>> Sent: Thursday, February 20, 2014 12:18:44 PM
>>>>> Subject: Re: [PATCH] Move CodeGenPrepare into lib/CodeGen
>>>>> 
>>>>> Hi All,
>>>>> 
>>>>>>> I agree, I don't see why we're conflating all of these different
>>>>>>> things. Can we not move the pass into CodeGen, and then deal with
>>>>>>> adding IR-pass-driving features into llc as a follow-up matter?
>>>>>>> 
>>>>> 
>>>>> Absolutely. Sorry - I should have been clearer, but I'm all for this.
>>>>> I'm just suggesting that making opt depend on CodeGen should be a
>>>>> temporary hack while we teach llc how to run IR -> IR passes neatly.
>>>>> Then the CGP tests should be updated to use llc rather than opt.
>>>> 
>>>> opt already depends on CodeGen so that it can use TTI, this is necessary to sensibly run the vectorizer, etc. and that seems unlikely to change.
>>>> 
>>>> -Hal
>>>> 
>>>>> 
>>>>> - Lang.
>>>>> 
>>>> 
>>>> --
>>>> Hal Finkel
>>>> Assistant Computational Scientist
>>>> Leadership Computing Facility
>>>> Argonne National Laboratory
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140221/6df2aad2/attachment.html>