ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D132352#3755355 <https://reviews.llvm.org/D132352#3755355>, @nikic wrote:
> 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. I think the key point here is whether or not "thread_id" is part of "memory". According to https://discourse.llvm.org/t/address-thread-identification-problems-with-coroutine/62015/48, we agree to treat "thread_id" is not part of memory. I feel the idea is to make these attributes more composable. (@nhaehnle ) And it looks like @jyknight @fhahn @rjmccall @efriedma tend to agree the direction if I don't misread. And your proposed solution should be available too. I think we need to get in consensus that whether or not "thread_id" is part of the "memory". And another benefit of this method is that it is helpful to solve the potential similar problem in green threads (which is called stackful coroutines, or fibers). We mention about it here: https://discourse.llvm.org/t/address-thread-identification-problems-with-coroutine/62015/28 (Some backgrounds for stackful coroutines: The stackful coroutines are not standard features and a vendor extension. the coroutine intrinsics in LLVM currently works for stackless coroutines. And the general implementation of stackful coroutine is not compiler dependent. The stackful coroutines save each register manually when switching. So it is hard to detect stackful coroutines in compiler.) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
