<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 18, 2013 at 8:23 AM, <span dir="ltr"><<a href="mailto:Andrea_DiBiagio@sn.scee.net" target="_blank">Andrea_DiBiagio@sn.scee.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">So..<br>
I have investigated more on how a new function attribute to disable<br>
optimization on a per-function basis could be implemented.<br>
At the current state, with the lack of specific support from the pass<br>
managers I found two big problems when trying to implement a prototype<br>
implementation of the new attribute.<br>
<br>
Here are the problems found:<br>
1) It is not safe to disable some transform passes in the backend.<br>
It looks like there are some unwritten dependences between passes and<br>
disrupting the sequence of passes to run may result in unexpected crashes<br>
and/or assertion failures;<br></blockquote><div><br></div><div>This sounds like a bug. It's probably worth bringing up as its own discussion on llvmdev if it is extremely prevalent, or file PR's (or send patches fixing it!) if it is just a few isolated cases.</div>
<div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) The fact that pass managers are not currently designed to support<br>
per-function optimization makes it difficult to find a reasonable way to<br>
implement this new feature.<br>
<br>
About point 2. the Idea that came in my mind consisted in making passes<br>
aware of the 'noopt' attribute.<br>
In my experiment:<br>
- I added a virtual method called 'mustAlwaysRun' in class Pass that<br>
'returns true if it is not safe to disable this pass'.<br>
If a pass does not override the default implementation of that method,<br>
then by default it will always return true (i.e. the pass "must<br>
always run" pass even when attribute 'noopt' is specified).<br>
- I then redefined in override that method on all the optimization passes<br>
that could have been safely turned off when attribute noopt was present.<br>
In my experiment, I specifically didn't disable Module Passes;<br>
- Then I modified the 'doInitialize()' 'run*()' and 'doFinalize' methods<br>
in Pass Manger to check for both the presence of attribute noopt AND the<br>
value returned by method 'mustAlwaysRun' called on the current pass<br>
instance.<br>
<br>
That experiment seemed to "work" on a few tests and benchmarks.<br>
However:<br>
a) 'noopt' wouldn't really imply no optimization, since not all codegen<br>
optimization passes can be safely disabled. As a result, the assembly<br>
produced for noopt functions had few differences with respect to the<br>
assembly generated for the same functions at -O0;<br>
b) I don't particularly like the idea of making passes "aware" of the<br>
'noopt' attribute. However, I don't know if there is a reasonable way to<br>
implement the noopt attribute without having to re-design how pass<br>
managers work.<br></blockquote><div><br></div><div>A redesign of the pass manager has been on the table for a while and the need seems more pressing daily. Definitely make sure that this use case is brought up in any design discussions for the new pass manager.</div>
<div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
c) Because of a. and b., I am concerned that a change like the one<br>
described above won't be accepted. If so however, I would be really<br>
interested in the feedback from the community. Maybe there are better ways<br>
to implement 'noopt' which I don't know/didn't think about?<br>
<br>
As I said, I am not very happy with the proposed solution and any feedback<br>
would be really appreciated at this point.<br>
<br>
By the way, here is how I thought the 'noopt' proposal could have been<br>
contributed in terms of patches:<br>
<br>
[LLVM IR][Patch 1]<br>
================<br>
This patch extends the IR adding a new attribute called 'noopt'.<br>
Below, is a sequence of steps which describes how to implement this patch.<br>
<br>
1) Add a definition for attribute 'noopt' in File llvm/IR/Attribute.h;<br>
2) Teach how attribute 'noopt' should be encoded and also how to print it<br>
out<br>
as a string value (File lib/IR/Attributes.cpp);<br>
2b) Add a new enum value for the new attribute in enum LLVMAttribute<br>
(File "include/llvm-c/Core.h");<br>
3) The new attribute is a function attribute;<br>
Teach the verifier pass that 'noopt' is a function attribute;<br>
Add checks in method VerifyAttributeTypes() (File lib/IR/Verifier.cpp):<br>
* NoOpt is a function-only attribute;<br>
* Assert if NoOpt is used in the same context as alwaysinline;<br>
* Assert if NoOpt is used in the same context as OptimizeForSize<br>
(needed?);<br>
* Assert if NoOpt is used in the same context as MinSize (needed?).<br>
4) Add a LLVM test in test/Feature to verify that we correctly disassemble<br>
the new function attribute (see for example file cold.ll);<br>
5) Teach the AsmParser how to parse the new attribute:<br>
* Add a new token for the new attribute noopt;<br>
* Add rules to parse the new token;<br>
6) Add a description of the new attribute in "docs/LangRef.rst";<br>
<br>
[LLVM][Opt][Patch 2]<br>
==================<br>
This patch implements the required changes to passes and pass managers.<br>
Below, is a sequence of steps which describes how to implement this patch.<br>
<br>
1) Make the new inliner aware of the new flag.<br>
* In lib/Transforms/IPO/Inliner.cpp:<br>
** do not inline the callee if it is not always_inline and the caller<br>
is marked 'noopt'.<br>
* No other changes are required since 'noopt' already implies 'noinline'.<br>
2) Tell the pass manager which transform passes can be safely disabled<br>
with 'noopt'.<br>
<br>
[CLANG][Patch 3]<br>
===============<br>
This patch teaches clang how to parse and generate code for functions that<br>
are marked with attribute 'noopt'.<br>
<br>
1) Lex<br>
* Add a new token for the 'noopt' keyword.<br>
* That keyword is for a function attribute.<br>
2) Sema<br>
* Add a rule to handle the case where noopt is passed as function<br>
attribute.<br>
* check that the attribute does not take extra arguments.<br>
* check that the attribute is associated to a function declaration.<br>
* Add the attribute to the IR Set of Attributes.<br>
3) CodeGen<br>
* noopt implies 'noinline.<br>
* noopt always wins over always_inline<br>
* noopt does not win over 'naked': naked functions only contain asm<br>
statements. This attribute is only valid for ARM, AVX, MCORE, RL78, RX<br>
and<br>
SPU to indicate that the specified function does not need<br>
prologue/epilogue<br>
sequence generated by the compiler. (NOTE: this constraint can be<br>
removed).<br>
4) Add clang tests:<br>
* in test/Sema:<br>
** Verify that noopt only applies to functions. (-cc1 -fsyntax-only<br>
-verify)<br>
* in test/CodeGen:<br>
** Check that noopt implies noinline<br>
** Check combinations of noopt and noinline and always_inline<br>
<br>
<br>
Andrea Di Biagio<br>
SN Systems - Sony Computer Entertainment Group.<br>
<br>
Andrea DiBiagio/SN R&D/BS/UK/SCEE wrote on 25/06/2013 15:20:12:<br>
<br>
> From: Andrea DiBiagio/SN R&D/BS/UK/SCEE<br>
> To: Nick Lewycky <<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>><br>
> Cc: <a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a>, <a href="mailto:llvmdev@cs.uiuc.edu">llvmdev@cs.uiuc.edu</a><br>
> Date: 25/06/2013 15:20<br>
> Subject: Re: [LLVMdev] [RFC] add Function Attribute to disable<br>
optimization<br>
><br>
<div class="im HOEnZb">> Hi Nick,<br>
><br>
> > From: Nick Lewycky <<a href="mailto:nicholas@mxc.ca">nicholas@mxc.ca</a>><br>
</div><div class="im HOEnZb">> > > This proposal is to create a new function-level attribute which<br>
would tell<br>
> > > the compiler to not to perform any optimizing transformations on the<br>
> > > specified function.<br>
> ><br>
> > What about module passes? Do you want to disable all module passes in<br>
a<br>
> > TU which contains a single one of these? I'll be unhappy if we need to<br>
<br>
> > litter checks throughout the module passes that determine whether a<br>
> > given instruction is inside an unoptimizable function or not. Saying<br>
> > that module passes are exempt from checking the 'noopt' attribute is<br>
> > fine to me, but then somebody needs to know how to module passes (and<br>
> > users may be surprised to discover that adding such an annotation to<br>
one<br>
> > function will cause seemingly-unrelated functions to become less<br>
optimized).<br>
<br>
</div><div class="im HOEnZb">> Right, module passes are a difficult case.<br>
> I understand your point. I think ignoring the `noopt' attribute (or<br>
> whatever we want to call it) may be the best approach in this case:<br>
> it avoid the problems you describe but should still be sufficient<br>
> for the purposes we care about. I am currently studying the module<br>
> passes in more details to be certain about this.<br>
<br>
> Thanks for the useful feedback,<br>
</div><div class="HOEnZb"><div class="h5">> Andrea Di Biagio<br>
> SN Systems - Sony Computer Entertainment Group<br>
><br>
> > > The use-case is to be able to selectively disable optimizations when<br>
> > > debugging a small number of functions in a compilation unit to<br>
provide an<br>
> > > -O0-like quality of debugging in cases where compiling the whole<br>
unit at<br>
> > > anything less than full optimization would make the program run too<br>
> > > slowly. A useful secondary-effect of this feature would be to allow<br>
users<br>
> > > to temporarily work-around optimization bugs in LLVM without having<br>
to<br>
> > > reduce the optimization level for the whole compilation unit,<br>
however we<br>
> > > do not consider this the most important use-case.<br>
> > ><br>
> > > Our suggestion for the name for this attribute is "optnone" which<br>
seems to<br>
> > > be in keeping with the existing "optsize" attribute, although it<br>
could<br>
> > > equally be called "noopt" or something else entirely. It would be<br>
exposed<br>
> > > to Clang users through __attribute__((optnone)) or [[optnone]].<br>
> > ><br>
> > > I would like to discuss this proposal with the rest of the community<br>
to<br>
> > > share opinions and have feedback on this.<br>
> > ><br>
> > > ===================================================<br>
> > > Interactions with the existing function attributes:<br>
> > ><br>
> > > LLVM allows to decorate functions with 'noinline', alwaysinline' and<br>
> > > 'inlinehint'. We think that it makes sense for 'optnone' to<br>
implicitly<br>
> > > imply 'noinline' (at least from a user's point of view) and<br>
therefore<br>
> > > 'optnone' should be considered incompatible with 'alwaysinline' and<br>
> > > 'inlinehint'.<br>
> > ><br>
> > > Example:<br>
> > > __attribute__((optnone, always_inline))<br>
> > > void foo() { ... }<br>
> > ><br>
> > > In this case we could make 'optnone' override 'alwaysinline'. The<br>
effect<br>
> > > would be that 'alwaysinline' wouldn't appear in the IR if 'optnone'<br>
is<br>
> > > specified.<br>
> > ><br>
> > > Under the assumption that 'optnone' implies 'noinline', other things<br>
that<br>
> > > should be taken into account are:<br>
> > > 1) functions marked as 'optnone' should never be considered as<br>
potential<br>
> > > candidates for inlining;<br>
> > > 2) the inliner shouldn't try to inline a function if the call site<br>
is in a<br>
> > > 'optnone' function.<br>
> > ><br>
> > > Point 1 can be easily achieved by simply pushing attribute<br>
'noinline' on<br>
> > > the list of function attributes if 'optnone' is used.<br>
> > > point 2 however would probably require to teach the Inliner about<br>
> > > 'optnone' and how to deal with it.<br>
> > ><br>
> > > As in the case of 'alwaysinline' and 'inlinehint', I think 'optnone'<br>
> > > should also override 'optsize'/'minsize'.<br>
> > ><br>
> > > Last (but not least), implementing 'optnone' would still require<br>
changes<br>
> > > in how optimizations are run on functions. This last part is<br>
probably the<br>
> > > hardest part since the current optimizer does not allow the level of<br>
> > > flexibility required by 'optnone'. It seems it would either require<br>
some<br>
> > > modifications to the Pass Manager or we would have to make<br>
individual<br>
> > > passes aware of the attribute. Neither of these solutions seem<br>
> > > particularly attractive to me, so I'm open to any suggestions!<br>
> > ><br>
> > > Thanks,<br>
> > > Andrea Di Biagio<br>
> > > SN Systems - Sony Computer Entertainment Group<br>
> > ><br>
> > ><br>
> > ><br>
**********************************************************************<br>
> > > This email and any files transmitted with it are confidential and<br>
intended<br>
> > > solely for the use of the individual or entity to whom they are<br>
addressed.<br>
> > > If you have received this email in error please notify<br>
<a href="mailto:postmaster@scee.net">postmaster@scee.net</a><br>
> > > This footnote also confirms that this email message has been checked<br>
for<br>
> > > all known viruses.<br>
> > > Sony Computer Entertainment Europe Limited<br>
> > > Registered Office: 10 Great Marlborough Street, London W1F 7LP,<br>
United<br>
> > > Kingdom<br>
> > > Registered in England: 3277793<br>
> > ><br>
**********************************************************************<br>
> > ><br>
> > > P Please consider the environment before printing this e-mail<br>
> > > _______________________________________________<br>
> > > LLVM Developers mailing list<br>
> > > <a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu" target="_blank">http://llvm.cs.uiuc.edu</a><br>
> > > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br>
> > ><br>
> ><br>
><br>
><br>
> **********************************************************************<br>
> This email and any files transmitted with it are confidential and<br>
> intended solely for the use of the individual or entity to whom they<br>
> are addressed. If you have received this email in error please<br>
> notify <a href="mailto:postmaster@scee.net">postmaster@scee.net</a><br>
> This footnote also confirms that this email message has been checked<br>
> for all known viruses.<br>
> Sony Computer Entertainment Europe Limited<br>
> Registered Office: 10 Great Marlborough Street, London W1F 7LP, United<br>
Kingdom<br>
> Registered in England: 3277793<br>
> **********************************************************************<br>
><br>
> P Please consider the environment before printing this e-mail<br>
<br>
**********************************************************************<br>
This email and any files transmitted with it are confidential and intended<br>
solely for the use of the individual or entity to whom they are addressed.<br>
If you have received this email in error please notify <a href="mailto:postmaster@scee.net">postmaster@scee.net</a><br>
This footnote also confirms that this email message has been checked for<br>
all known viruses.<br>
Sony Computer Entertainment Europe Limited<br>
Registered Office: 10 Great Marlborough Street, London W1F 7LP, United<br>
Kingdom<br>
Registered in England: 3277793<br>
**********************************************************************<br>
<br>
P Please consider the environment before printing this e-mail<br>
_______________________________________________<br>
</div></div><div class="HOEnZb"><div class="h5">cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</div></div></blockquote></div><br></div></div>