[llvm-commits] [PATCH] treat calls to noreturn functions as side effecting

[Nick Lewycky]

Duncan Sands wrote:
> Hi Nuno,
>
> On 23/07/12 17:47, Nuno Lopes wrote:
>> Quoting Duncan Sands<baldrick at free.fr>:
>>
>>> Hi Chandler,
>>>
>>>> Anyways, LLVM, much to my regret, decided to make infinite loops a magical
>>>> observable side-effect, and not delete them. One of the consequences of this
>>>> decision is that there is one of two bugs in the current system now:
>>>>
>>>> 1) functionattrs shouldn't mark functions with infinite loops as readonly, or
>>>> 2) noreturn functions should have the same "observable" property as infinite
>>>> loops.
>>>>
>>>> As there are many noreturn functions which do not have infinite loops and to
>>>> which this magical property really shouldn't apply, I strongly prefer #1. It
>>>> also clarifies the cost of this treatment of infinite loops -- they are
>>>> implicitly a side-effect carrying construct.
>>>>
>>>> So in essence, I think this is a bug in functionattrs which stems from the
>>>> decision to not delete infinite loops.
>>>
>>> I disagree.  Read-only is what it says: the function only reads memory, doesn't
>>> write it.  What has that to do with infinite loops?  I would rather see another
>>> attribute, call it "finite", for functions that can be proved to complete in
>>> finite time.  Then a function call with no uses can be deleted if read-only,
>>> nounwind and finite.
>>
>> I agree with Duncan.
>> Overloading readonly is not the way to go, since their usage is quite different.
>> For example, alias analysis cares about pointers not escaping (and
>> readonly/readnone are useful to prove that), but it doesn't care about
>> non-terminating functions. BTW, readnone functions may not terminate either.
>> That said, computing termination of loops is a really tricky business.
>
> this is probably why no one implemented this :)

But I have implemented this: 
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20100705/103998.html

One of the truly unfortunate quirks in the implementation is that 
FunctionAttrs is a CGSCC pass and we need SCEV which is a FunctionPass, 
and that doesn't work. Chandler is working on fixing that. The patch in 
that link actually handles it by having a dumb ModulePass analysis that 
uses SCEV, and that FunctionAttrs uses. Lunacy.

Chris was concerned that it we wouldn't be able to solve it much of the 
time, as seen in this paragraph:

 > The other thing that concerns me is that there is a lot of cases that
 > really can't be handled.  Iterating over an std::map or another 
node-based
 > container can't be handled with this.  If you have some sort of front-end
 > attributes (like the halting attribute you proposed for clang) or other
 > language semantics that would be useful, then it makes sense.

but I think that's something we have to live with. I eventually gave up 
on the thread because I wasn't able to easily find examples of things 
that were broken with the current status quo.

Nick

   However I'm pretty sure the
> loop analysis can provide this information in straightforward cases, and other
> cases probably don't matter (i.e. not marking them "finite" has no important
> adverse impact).
>
>    We can
>> use SCEV to give a hand, but... Moreover, the termination property may depend on
>> the inputs given to the function.
>> AFAIR, GCC's pure/const function attributes imply terminating behavior.
>
> Yes, gcc-4.7 will delete calls to pure/const functions just based on knowing
> that they are pure/const.
>
> A simple first step to implementing this would be to add a "finite" attribute
> and (in SimplifyLibCalls) add it to library functions that are "finite", which
> probably means all of them, and in clang add it to functions that are known to
> the front-end to be pure or const, eg because someone plunked an
> __attribute__((pure)) on it.
>
> The next step would be to add a pass that computes the "finite" attribute.  To
> begin with it could do it only for functions that contain no loops.  For those
> it can simply check that all called functions are marked "finite", and if so
> mark this one "finite" too.
>
> Ciao, Duncan.
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>