[PATCH] D132352: Introduce noread_thread_id to address the thread identification problem in coroutines

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

Okay, this is a bit tricky because we have three different things:

1. The noread_thread_id attribute, the lack of which was causing issues with intrinsics in the previous version
2. The meaning of the readnone (etc) attributes, which for pragmatic reasons has to exclude thread IDs for now
3. The meaning of doesNotReadMemory() etc queries, which in the previous version included thread ID accesses, but in the new version require a separate call

I think my question here would be why this did not stick with the previous implementation approach that also affects doesNotReadMemory and AA queries (and thus makes everything "automatically correct"), and only added the noread_thread_id attribute to make intrinsic handling more precise?

My general vision for this area was that after D130896 <https://reviews.llvm.org/D130896>, we would add ThreadID as an additional ModRef location, which gets removed for non-presplit-coroutines due to being constant. This would follow the interpretation that the thread ID is part of "memory" though, which kind of goes against the approach here.


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

https://reviews.llvm.org/D132352