[llvm-dev] [RFC] Coroutine and pthread_self

[Nicolai Hähnle via llvm-dev]

On Wed, Nov 25, 2020 at 12:06 AM Joerg Sonnenberger via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> On Tue, Nov 24, 2020 at 04:52:32PM -0500, James Y Knight wrote:
> > In any case, what it lowers to in Clang is the LLVM IR function-attribute
> > readnone, which I'd argue is even more clearly correct to use on an LLVM
> > function returning the address of a TLS global variable. Note that llvm
> > infers that property via the FunctionAttrs pass, e.g. for this function:
> > @t = thread_local global i32 0, align 4
> > define i32* @foo() {
> > ret i32* @t
> > }
>
> I don't see how that is a valid transformation under the definition in
> the language reference either. I also don't believe that this is the
> only situation it can happen, I would expect OpenMP to expose similar
> issues.

I tend to agree with this. The LangRef definition of readnone is:

"On a function, this attribute indicates that the function computes
its result (or decides to unwind an exception) based strictly on its
arguments, without dereferencing any pointer arguments or otherwise
accessing any mutable state (e.g. memory, control registers, etc)
visible to caller functions."

The tricky part is that the "thread self identity" is a piece of state
that used to be immutable, but with the introduction of coroutines, it
has become mutable. The example @foo reads the "thread self identity"
piece of state. As long as that state was immutable, it was correct to
mark the function as readnone. Now that the state has become mutable,
this is no longer correct.

That may seem annoying, but I suspect it's best to just swallow that
pill and take it to its logical conclusion.

Doing so ends up treating pthread_self(), @foo, etc. more
conservatively, but correctly. Treating them correctly and less
conservatively is really a kind of alias analysis problem. I would
hesitate to just add a new ad-hoc attribute for it, we already have so
many of those, and would prefer adding a mechanism for this kind of
thing generically.

We faced a similar problem in defining SPIR-V / Vulkan extensions for
raytracing, where part of a "thread's self identity" can change for
very similar reasons at "invocation repack instructions".[0] So just
like @llvm.coro.suspend can change the state read by pthread_self(),
which used to be immutable before coroutines were introduced,
invocation repack instructions can change pieces of state that used to
be immutable before raytracing pipelines were introduced.[1]

It seems to me that what we have here are different kinds of
non-addressable pieces of state which most code doesn't write to, but
some code may read from. Perhaps there should be a parameterized
readnone(x) attribute, where x identifies the kind of state, and a
corresponding writes(x) attribute, which indicates that a function may
write to the state identified by x? Or perhaps we should just say that
pthread_self() and similar should be inaccessiblememonly, but that
doesn't allow us to capture the fact that most code doesn't _write_ to
the relevant "inaccessible memory"...

Cheers,
Nicolai

[0] You can search for that term in a recent version of the Vulkan
spec that includes KHR extensions, e.g.
https://www.khronos.org/registry/vulkan/specs/1.2-khr-extensions/html/vkspec.html.

[1] To make matters more fun, Vulkan has an analog of thread_local
variables (the Private storage class). Pointers are kind of weird in
Vulkan, but assuming that will change eventually and we will be able
to take pointers of such variables generically, then I believe unlike
@llvm.coro.suspend, invocation repack instructions would not affect
the values of such pointers. So there are really two different kinds
of hidden state in play here.

>
> Joerg
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev



-- 
Lerne, wie die Welt wirklich ist,
aber vergiss niemals, wie sie sein sollte.