[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 1:34 PM, Eric Christopher <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.
I’m fine with reverting this if you think it is fine to expose this cl::opt as a global in a public header?

— 
Mehdi



> 
> 
>  
> 
> 
>>  
>> Modified: llvm/trunk/include/llvm/Target/TargetOptions.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetOptions.h?rev=275348&r1=275347&r2=275348&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Target/TargetOptions.h?rev=275348&r1=275347&r2=275348&view=diff>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Target/TargetOptions.h (original)
>> +++ llvm/trunk/include/llvm/Target/TargetOptions.h Wed Jul 13 18:39:46 2016
>> @@ -100,7 +100,8 @@ namespace llvm {
>>            DisableIntegratedAS(false), CompressDebugSections(false),
>>            RelaxELFRelocations(false), FunctionSections(false),
>>            DataSections(false), UniqueSectionNames(true), TrapUnreachable(false),
>> -          EmulatedTLS(false), FloatABIType(FloatABI::Default),
>> +          EmulatedTLS(false), EnableIPRA(false),
>> +          FloatABIType(FloatABI::Default),
>>            AllowFPOpFusion(FPOpFusion::Standard), Reciprocals(TargetRecip()),
>>            JTType(JumpTable::Single), ThreadModel(ThreadModel::POSIX),
>>            EABIVersion(EABI::Default), DebuggerTuning(DebuggerKind::Default),
>> @@ -207,6 +208,9 @@ namespace llvm {
>>      /// function in the runtime library..
>>      unsigned EmulatedTLS : 1;
>> 
>> +    /// This flag enables InterProcedural Register Allocation (IPRA).
>> +    unsigned EnableIPRA : 1;
>> +
>> 
>> You forgot to add it to the copy routine.
> 
> Did you mean comparison? (I don’t see any copy routine)
> 
> r275467
> 
> Thanks,
> 
> — 
> Mehdi

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160714/83f9c102/attachment.html>