[llvm] r189101 - Add function attribute 'optnone'.

[Nick Lewycky]

Andrea_DiBiagio at sn.scee.net wrote:
> Hi Duncan,
>
> Sorry if I am late with this reply..
>
>> From: Duncan Sands<duncan.sands at gmail.com>
>> Hi Andrea,
>>>> +``optnone``
>>>> +    This function attribute tells the optimizers not to modify the
> body
>>> of
>>>> +    the function.  The optimizers are still allowed to analyze the
>>> function,
>>>> +    for example to determine if it writes to memory, and to delete
> it
>>> if it
>>>> +    is unused.  But the instructions composing the function should
> be
>>> left
>>>> +    alone.
>>>
>>> I think that rewriting the description of the optnone attribute as you
>>> suggested is ok and it is definitely an improvement over my previous
>>> description.
>>>
>>> As you said, as long as the instructions within the optnone functions
> are
>>> left unchanged, it will always be possible to analyze them to simplify
>>> code from other functions etc.
>>> I think the goal is to avoid the case where the presence of optnone
>>> functions in a module negatively affect how other functions are
> optimized.
>>
>> I don't think this is achievable.  If no optimizations have been run on
>> a function, most analyses will do a poor job on it.  And disabling
> inlining
>> of optnone functions clearly impacts optimization of the functions it
> would
>> otherwise have been inlined into.
>
> True, what I wrote is wrong.
> There will still be a negative effect on other functions; however it
> should be better
> than the case where we don't analyze optnone functions at all.
>
>>> I agree that a GlobalDCE pass should be allowed to remove a dead
> function
>>> even if it marked optnone.
>>>
>>> My question is whether Module Passes like the DAE Pass (Dead Argument
>>> Elimination pass) should be allowed to remove unused arguments from
>>> optnone functions. I personally don't have a strong opinion on this;
>>
>> I also don't have an opinion on this.  It might be helpful to understand
>> better what optnone is good for, i.e. who wants this and why.
>>
>>> however if we allow DAE to remove unused arguments, then we should
> decide
>>> what to do with other Module Passes as well.
>>> For example, is it ok to allow the IPCP (InterProcedural Constant
>>> Propagation pass) to propagate constants inside optnone functions?
>>
>> I don't think doing IPCP makes any sense: if ordinary constant folding
>> within the function has been disabled, why should IPCP be enabled?
>>
>>> If we don't allow it then we preserve the instructions in the optnone
>>> function, however we may not spot opportunities for removing unused
>>> arguments when running the DAE Pass.
>>
>> There are many other things that would get in the way of DAE, eg that
>> simplifycfg hasn't zapped dead basic blocks that use the argument, that
>> instcombine hasn't zapped dead uses of the argument etc etc.  Basically
>> DAE isn't going to be able to do anything except in the most trivial
>> cases, and I think we're just going to have to live with that.
>
> Attribute optnone was originally intended to be used mainly for debugging
> purposes.
> Our customers almost exclusively use optimized builds because unoptimize
> builds are too slow for their purposes, but the debug experience on
> optimized builds is very often not good enough. Applying attribute optnone
> to a function would allow the user to debug that function as if it were an
> unoptimized build but having the rest of the program fully optimized. With
> this use case in mind, I originally decided to make the 'optnone' behavior
> similar to -O0.

Thank you! Finally I understand the motivation behind "optnone" well 
enough to make informed decisions about its design!

Firstly, the real way to fix this is to solve the optimized+debugging 
experience, but of course that's not happening any time soon enough. You 
need something now, but it does point out that maybe optnone isn't 
something we want to have in llvm forever -- it's just a workaround for 
a worse problem.

I think you can build this out of existing pieces, or optionally with a 
couple of extensions. The frontend needs an 'optnone' attribute. When 
codegen'ing such a function, it should apply the 'noinline' attribute to 
it, and also to each of the call or invoke instructions that it contains.

In LLVM, you need to control two pieces. Your frontend already controls 
the pass manager, at least to the extent that its API will permit. If 
you don't want to run module passes or CGSCC passes, you can just 
iterate over the list of functions and skip the functions you don't want 
to optimize and just run over the other ones. Or you could make a small 
extension to add a "skipFunctionList" to the pass manager, but really 
you can make that yourself: remove the function's body from the function 
before running the pass manager. Yep. Splicing a functions's blocks into 
a parentless function will leave a declaration in place in the module at 
the same pointer address. Then you put it back after you've run the 
IR-level pass managers. It's even an O(1) operation.

Finally, you also need to ensure that the backend emits at -O0. The 
TargetMachine stores some options (TargetOptions) on a per-function 
basis. Make the codegen opt level one of those. (This may be somewhat 
difficult, you could run everything through the -O0 codegen when 
anything is 'optnone' -- but then, your existing 'optnone' attribute is 
in the exact same situation.)

Nick

>
> About the DAE:
> I agree that the DAE would end up removing unused arguments only in
> trivial cases.
> This may still be acceptable from a debugging perspective since it
> wouldn't really negatively affect the user debugging experience (i.e. the
> user simply will not be able to "see" dead argument).
> However, if the plan is to achieve a behavior similar to -O0, then it may
> still be better to not allow DAE to change optnone functions.
>
>>>
>>>>> +    This attribute cannot be used together with the
> ``alwaysinline``
>>>>> +    attribute; this attribute is also incompatible
>>>>> +    with the ``minsize`` attribute and the ``optsize`` attribute.
>>>>> +
>>>>> +    The inliner should never inline this function in any situation.
>>>>> +    Only functions with the ``alwaysinline`` attribute are valid
>>>>> +    candidates for inlining inside the body of this function.
>>>>
>>>> Ha, "alwaysinline" strikes again!  What happens when the unmovable
>>>> "optnone" meets the irrestistable "alwaysinline"?  Maybe the compiler
>>>> should just error out in this case: declare that this combination is
>>>> invalid and have the verifier catch it.
>>>
>>> Yes, those attributes are incompatible and r189101 introduces rules in
> the
>>> Verifier to catch when attribute 'alwaysinline' is specified when
>>> 'optnone' is also present.
>>> If for example I define a function with both attributes optnone and
>>> alwaysinline, then the verifier will be able to catch it and print out
>>> that "Attributes 'alwaysinline and optnone' are incompatible!".
>>
>> I meant: what if the optnone function calls a function marked
> alwaysinline.
>> Inlining it into the optnone function changes the code of the optnone
>> function, so is contrary to the principal that optnone function bodies
> are
>> just left alone.
>
> The idea is trying to get 'optnone' as close as possible to the -O0
> behavior.
> This is the main reason why I originally decided to allow the inlining of
> alwaysinline
> functions inside optnone functions.
>
> I think the documentation should be improved to emphasize that -O0
> behavior.
>
> Andrea
>
> **********************************************************************
> This email and any files transmitted with it are confidential and intended
> solely for the use of the individual or entity to whom they are addressed.
> If you have received this email in error please notify postmaster at scee.net
> This footnote also confirms that this email message has been checked for
> all known viruses.
> Sony Computer Entertainment Europe Limited
> Registered Office: 10 Great Marlborough Street, London W1F 7LP, United
> Kingdom
> Registered in England: 3277793
> **********************************************************************
>
> P Please consider the environment before printing this e-mail
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>