[PATCH] D101701: [nofree] Refine concurrency requirements

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D101701#2746841 <https://reviews.llvm.org/D101701#2746841>, @nlopes wrote:

> @jdoerfert a function that syncs with another thread may not free anything itself and the other thread may not free anything either.
> If you require an optimization to have nofree + nosync in a call to preserve dereferenceability then you can't optimize this case.

I disagree, and I think this is the conceptual differences we have here.

If one does only look at a function in isolation then I agree, `nofree` alone is not sufficient to make statements about all call sites and arguments.
However, if you look at a call site of a `nofree` function the situation is different because you have context information about the arguments.

Let's go back to my last example:

  a = malloc
  b = malloc
  unknown(a);                  // no attributes 0-> escapes
  nofree_nocapture_only(a, b); // `nofree`, `nocapture` on args

I know that at the end of this sequence `b` has not been freed, even though there is no `nosync` in sight.
I can also derive `nofree` for `nofree_nocapture_only` without worrying about atomics, for example.
(Here is the sequence in action with heap2stack kicking in for b: https://godbolt.org/z/hjYrhaEvW)

So, with `nofree` and `nosync` being completely separated, as it was in the very beginning, you can:

1. Optimize even if only one is present, given additional call site information.
2. Derive `nofree` even if `nosync` is not present.

All the "strong `nofree`" effectively gives us is that it combines the to attributes. 
Let's introduce a helper:
`bool cannotFreeAtAll(Function &F) { return F.hasAttr(NoFree) && F.hasAttr(NoSync); }` 
you get your guarantee that `F` will not, in any way` free something. That said,
I find the call site specific reasoning is the way to go anyway but if some generic
"for all call sites and arguments" handling is required we can go with the helper instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101701