[PATCH] D101701: [nofree] Refine concurrency requirements

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

In D101701#2754739 <https://reviews.llvm.org/D101701#2754739>, @jdoerfert wrote:

> In D101701#2754648 <https://reviews.llvm.org/D101701#2754648>, @nlopes wrote:
>
>>>> 3. Today's `nofree` enables annotations and optimizations that the proposed change would no longer allow. I can't say for certain whether this is false, but at least the examples given so far seem to fail (the first one because you can just use the `nofree` **argument attribute** instead, and the second is discussed above in this comment).
>>>
>>> I think once I manage to explain the examples you will reconsider this point. To give you another one:
>>>
>>>   __attribute__((nosync)) // or __attribute__((assume("nosync")) or #pragma omp assume ext_no_synchronization
>>>   void user_code() {
>>>     a = malloc();
>>>     b = malloc();
>>>     unknown_function_with_a_now_nosync_call_site(a, b);
>>>     ...
>>>     free(a);
>>>   }
>>
>> This is a good example, that shows that if the user annotates a function with nosync then inferring nofree becomes easier. So the proposed semantics doesn't require nosync, but inference can be aided by its presence (not in this patch yet).
>
> This is not at all what the example shows. The example shows that we cannot infer nofree for `unknown_function_with_a_now_nosync_call_site` with the new semantics but with the old semantics we could.
> Only with the existing semantics you can optimize this example, not with the new one. I would have assumed this is implied given that I provided the example and I am arguing the proposed semantics are useless and prevent optimizations.

For the example above, I don't see how you can deduce nofree for user_code or for unknown_function_with_a_now_nosync_call_site without further information that is not given in this example. Without knowing anything else, unknown_function_with_a_now_nosync_call_site may free some pointer stored in a global var. Who knows?
If my reading is wrong, please make the example more explicit and explain things more slowly, otherwise I'm unable to understand it. These things are hard and nothing is implied or implicit.

>> The semantics proposed in this patch makes a lot of sense to me. And to make things clear, this proposal *does not* make nofree imply nosync (nor the other way around).
>
> For deduction purposes it effectively does imply/require nosync. If you disagree, please provide an example where we can derive nofree without nosync under the proposed semantics.

I already gave an example that shows that the semantics proposed in this patch is more expressive that the old one. Please don't mix semantics and some inference algorithm you have in mind (that hasn't been shared with us).
A simple example: consider a program with 2 threads, a main one and a helper thread. The helper thread doesn't free anything. The main thread may sync with the helper thread, so some function are not nosync, but you can still tag them as nofree if they don't free up anything.
So the proposed semantics is strictly more expressive than the old one. Plus it doesn't make it any harder to infer than before.

Even if the current patch cannot infer nofree for the example I've provided, there's nothing preventing someone writing an algorithm that could infer that in future. Plus, using your own argument, users may provide that information.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101701/new/

https://reviews.llvm.org/D101701