[llvm] r275348 - Add EnableIPRA to TargetOptions, and move the cl::opt -enable-ipra to TargetMachine.cpp

[Mehdi Amini via llvm-commits]

> On Jul 14, 2016, at 10:34 PM, Eric Christopher <echristo at gmail.com> wrote:
> 
> 
> 
> On Thu, Jul 14, 2016 at 2:01 PM Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
>> On Jul 14, 2016, at 1:34 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>> 
>> 
>> 
>> On Thu, Jul 14, 2016 at 1:30 PM Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
>>> On Jul 14, 2016, at 1:26 PM, Eric Christopher <echristo at gmail.com <mailto:echristo at gmail.com>> wrote:
>>> 
>>> 
>>> 
>>> On Wed, Jul 13, 2016 at 4:47 PM Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>>> Author: mehdi_amini
>>> Date: Wed Jul 13 18:39:46 2016
>>> New Revision: 275348
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=275348&view=rev <http://llvm.org/viewvc/llvm-project?rev=275348&view=rev>
>>> Log:
>>> Add EnableIPRA to TargetOptions, and move the cl::opt -enable-ipra to TargetMachine.cpp
>>> 
>>> Avoid exposing a cl::opt in a public header and instead promote this
>>> option in the API.
>>> Alternatively, we could land the cl::opt in CommandFlags.h so that
>>> it is available to every tool, but we would still have to find an
>>> option for clang.
>>> 
>>> 
>>> Are you planning to have it as a -f option to clang or just something via -mllvm?
>> 
>> 
>> Not sure yet, what do you suggest?
>> 
>> I'd probably leave it as a -mllvm option at the moment which means you don't need to put it into TargetOptions. Since you probably only would need it for llc anyhow?
> 
> The question is “how to share access to this option as it starts to affect various part of the codegen”. 
> I was not really happy with the previous state which was adding a global cl::opt into TargetPassConfig.h (and pull CommandLine.h into this public header as well), and adding it to “TargetOptions” seemed cleaner.
> 
> Seems cleaner than that for sure.
>  
> I’m fine with reverting this if you think it is fine to expose this cl::opt as a global in a public header?
> 
> 
> What are you envisioning ultimately? Staged rollout per target? Perhaps something similar to the MI Scheduler then?

Yes an opt-in staged rollout per target seems likely. I’m not familiar with the MI Scheduler, is it just an option on TargetSubtargetInfo?
How do we enable/disable it on the command line during development?

— 
Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160715/8af22ae3/attachment.html>