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

ChuanqiXu added a comment. We prefer https://reviews.llvm.org/D135550 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

ChuanqiXu added a comment. We prefer https://reviews.llvm.org/D135550 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

ChuanqiXu added a comment. @nhaehnle @jyknight @nikic @efriedma @fhahn ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

rjmccall added a comment. In D132352#3757433 , @ChuanqiXu wrote: > In D132352#3757415 , @rjmccall > wrote: > >> Stackful coroutine bodies should be straightforward to support on top of the >> other work you've

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

ChuanqiXu added a comment. In D132352#3757415 , @rjmccall wrote: > Stackful coroutine bodies should be straightforward to support on top of the > other work you've been doing, if anyone's actually interested in pursuing > them. As far as the optimizer

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

rjmccall added a comment. Stackful coroutine bodies should be straightforward to support on top of the other work you've been doing, if anyone's actually interested in pursuing them. As far as the optimizer needs to know, a stackful coroutine function is just like a presplit stackless

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

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In 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

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

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

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

rjmccall added a comment. Seems okay to me, but like I said, it'd be good to get AA eyes on it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 ___ cfe-commits mailing list

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

ChuanqiXu marked an inline comment as done. ChuanqiXu added inline comments. Comment at: llvm/include/llvm/IR/InstrTypes.h:1863 + /// not access or only reads memory. + bool doesNotReadThreadIDNorLivesInPresplitCoroutine() const { +return doesNoReadThreadID() ||

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

ChuanqiXu updated this revision to Diff 455209. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 Files: llvm/docs/LangRef.rst llvm/include/llvm/Bitcode/LLVMBitCodes.h

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

rjmccall added inline comments. Comment at: llvm/include/llvm/IR/InstrTypes.h:1863 + /// not access or only reads memory. + bool doesNotReadThreadIDNorLivesInPresplitCoroutine() const { +return doesNoReadThreadID() || !getFunction() || rjmccall wrote: >

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

ChuanqiXu updated this revision to Diff 454697. ChuanqiXu added a comment. I post the incorrect version the last time. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 Files: llvm/docs/LangRef.rst llvm/include/llvm/Bitcode/LLVMBitCodes.h

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

ChuanqiXu updated this revision to Diff 454696. ChuanqiXu marked 3 inline comments as done. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/ https://reviews.llvm.org/D132352 Files: llvm/docs/LangRef.rst

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

rjmccall added a comment. Okay. Doc parts LGTM, but I don't Comment at: llvm/include/llvm/IR/InstrTypes.h:1855 + } + void setNoReadThreadID() { addFnAttr(Attribute::NoReadThreadID); } + The closest existing precedent would suggest `doesNotReadThreadID` and

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

ChuanqiXu updated this revision to Diff 454683. ChuanqiXu edited the summary of this revision. ChuanqiXu added a comment. Address comments: - Edit LangRef.rst. - Split the add-the-attributes part in later revisions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132352/new/

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

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2260 Result.addFnAttr(llvm::Attribute::ReadNone); -else if (ReadOnly) + Result.addFnAttr(llvm::Attribute::NoReadThreadID); +} else if (ReadOnly) Hmm, my comment here got

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

rjmccall added inline comments. Comment at: llvm/docs/LangRef.rst:1931 +This attribute indicates that the function does not read thread id. +The behavior of noread_thread_id shouldn't depend on the thread it lives in. + Suggestion: > This attribute