[llvm-dev] [RFC] Coroutine and pthread_self

[James Y Knight via llvm-dev]

I view what happened is that we've introduced effectively a new form of IR
(pre-coroutine-split IR), yet when doing so, we failed to realize
the impact that a "suspend" allowing "thread identity" to change would have
on the rest of LLVM. So, oops! And now we need to design what things really
should be looking like here going forward.

Let me try to make the two proposals so far more concrete.

Assumptions:
- We're not going to move CoroSplit early in the pipeline, so this issue
really does need to be solved.
- We should not change the meaning of the C `attribute((const))` to
prohibit reading the thread-identity, but, we can change what LLVM IR that
transforms into.
- As a prerequisite, we have already solved the TLS-address-is-a-constant
issue -- e.g. by moving TLS address lookup to an intrinsic (I have some
other comments about that issue, separately).

Proposal 1:
- The readnone attribute is redefined to prohibit depending on thread id.
- Create a new attribute "readthreadidonly", which is similar to readnone,
except that it may access the current thread identity.
- The new @llvm.threadlocaladdr intrinsic (or whatever it ends up as) is
given this attribute.
- Clang's `attribute((const))` emits this attribute, instead of readnone.
- Any other frontends which emit the "readnone" attribute may also need to
be updated and emit readthreadidonly instead of readonly.
- Optimization passes that are oblivious to the new readthreadidonly will
fail correct -- they won't *misoptimize* by moving such a call across an
llvm.coro.suspend call (good) -- but they also will regress performance of
straightline code (bad).
- To avoid regressing performance, we'll need to update all optimization
passes which currently handle readnone, to also handle readthreadidonly
similarly.
   - Option 1: (quick'n'dirty) treat readthreadidonly as readnone (allowing
arbitrary movement) on any function which doesn't contain an
llvm.coro.suspend, and treat readthreadidonly as inaccessiblememonly if
there are any suspends.
   - Option 2: actually understand that readthreadid can be moved anywhere
except across an llvm.coro.suspend call.

Proposal 2:
- The readnone attribute (continues to) allow depending on thread id.
- C frontend's `attribute((const))` continues to emit the readnone
attribute.
- The new @llvm.threadlocaladdr intrinsic is marked readnone.
- (Optional) add a not_threadid_dependent attribute, which says that a
function doesn't care which thread it's invoked on.
- Neither clang nor other frontends need to change which IR attributes
they emit.
- Optimization passes must be taught not to move readnone calls across
llvm.coro.suspend, or they will be incorrect.
   - Option 1: (quick'n'dirty) treat 'readnone' as 'inaccessiblememonly' in
any function which contains a llvm.coro.suspend.
   - Option 2: actually understand that readnone can move anywhere in the
function except across an llvm.coro.suspend call (unless
not_threadid_dependent is present, in which case it can also move across
that).

I'm still slightly in favor of proposal 2, but either seems OK. Both seem
like they'll be effectively the same amount of work to implement in LLVM. I
feel like having the "not_threadid_dependent" attribute be separate from
"readnone" (doesn't access memory) is logical and consistent -- and having
them be distinct may potentially also be useful in other circumstances.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20201210/716888fe/attachment.html>