[PATCH] D128695: [ODRHash diagnostics] Move `ODRDiagsEmitter` to libAST in separate files. NFC.

2022-07-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I see. It looks like the tricks in `.git-blame-ignore-revs` doesn't work for this kind of move. I admit I don't look these changes in details. But as long as it is helpful for future changes, I think it should be good. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/Scope.cpp:152-154 + // Consider the variable as NRVO candidate if the return slot is available + // for it in the current scope, or if it can be available in outer scopes. + NRVO = CanBePutInReturnSlot ? VD : nullptr;

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/Scope.cpp:152-154 + // Consider the variable as NRVO candidate if the return slot is available + // for it in the current scope, or if it can be available in outer scopes. + NRVO = CanBePutInReturnSlot ? VD : nullptr;

[PATCH] D124149: [NFC] follow up code cleanup after D123837

2022-05-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D124149#3500423 , @iains wrote: > sorry for being slow, concentrating on module initialisers! > > this LGTM (but we can still probably make the visibility code cleaner - > perhaps after we get the main functionality installe

[PATCH] D124149: [NFC] follow up code cleanup after D123837

2022-05-09 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGdebd9bf3f019: [NFC] follow up code cleanup after D123837 (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124149/new/ https://reviews.

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

2022-05-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 428263. ChuanqiXu added a comment. Code Cleanups CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 Files: clang/include/clang/AST/DeclBase.h clang/include/clang/Basic/LangOptions.def clang/include/cl

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

2022-05-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rsmith gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: aaron.ballman, cor3ntin, clang-language-wg. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. According to https://cplusplus.github.io/CWG/

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11244 +def err_coroutine_unusable_new : Error< + "'operator new' provided by %0 is not usable with the function signature of %1" +>; The error message here following t

[PATCH] D125521: [Diagnostic] Warn if the size argument of memset is character literal zero

2022-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added a reviewer: efriedma. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This tries to fix https://github.com/llvm/llvm-project/issues/55402 Repositor

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 429176. ChuanqiXu added a comment. Remove invalid character CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125517/new/ https://reviews.llvm.org/D125517 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang

[PATCH] D125521: [Diagnostic] Warn if the size argument of memset is character literal zero

2022-05-15 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3bef90dff64f: [Diagnostic] Warn if the size argument of memset is character literal (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 429595. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125517/new/ https://reviews.llvm.org/D125517 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Se

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked an inline comment as done. ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:1312 + + bool PromiseContainNew = [this, &PromiseType]() -> bool { +DeclarationName NewName = erichkeane wrote: > Slight preference to j

[PATCH] D125517: [Frontend] [Coroutines] Emit error when we found incompatible allocation function in promise_type

2022-05-16 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG452fac9534c0: [Frontend] [Coroutines] Emit error when we found incompatible allocation (authored by ChuanqiXu). Repository: rG LLVM Github Monorep

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

2022-05-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rjmccall @eli.friedman @jyknight gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291 ___ cfe-commits mailing list cfe-commi

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

2022-05-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D125291#3523818 , @efriedma wrote: > I don't really understand how this is supposed to interact with D125292 > ; even if you strip the readnone attribute > from the call instruction, we'll

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

2022-05-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 430583. ChuanqiXu added a comment. Address comments: - Lowering the introduced intrinsic in CodeGen pipelines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291 Files: clang/lib/CodeGen/CGExpr.cpp cla

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

2022-05-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 430585. ChuanqiXu added a comment. Cleanup codes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D125291/new/ https://reviews.llvm.org/D125291 Files: clang/lib/CodeGen/CGExpr.cpp clang/lib/CodeGen/ItaniumCXXABI.cpp clang/test/CodeGenCXX/cxx11

[PATCH] D125291: Introduce @llvm.threadlocal.address intrinsic to access TLS variable (1/3)

2022-05-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp:1 //===- PreISelIntrinsicLowering.cpp - Pre-ISel intrinsic lowering pass ===// // I feel this is a better place than SelectDAG. CHANGES SINCE LAST ACTION https:/

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

2022-05-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rsmith gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm.org/D113545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D119544: Deferred Concept Instantiation Implementation

2022-05-19 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D119544#3494281 , @erichkeane wrote: > Updated to include the fix for the libcxx issue, which was that we weren't > properly differentiating between 'friend' functions based on concepts. I'll > likely be submitting this e

[PATCH] D119544: Deferred Concept Instantiation Implementation

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D119544#3527556 , @erichkeane wrote: > In D119544#3526810 , @ChuanqiXu > wrote: > >> In D119544#3494281 , @erichkeane >> wrote: >> >>> Upd

[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added a reviewer: erichkeane. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This is the following update for the update in CWG issue 2585: https://cplu

[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 431558. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126187/new/ https://reviews.llvm.org/D126187 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaCoroutine.cpp clang/test/SemaCXX/coroutine

[PATCH] D126187: [C++20] [Coroutines] Conform the updates for CWG issue 2585

2022-05-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:1293 // that just takes the requested size. - - FunctionDecl *OperatorNew = nullptr; - FunctionDecl *OperatorDelete = nullptr; - FunctionDecl *UnusedResult = nullptr; - bool PassAlignment = false

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

2023-10-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D108905#4654393 , @smeenai wrote: > In D108905#2975712 , @rsmith wrote: > >> No decision as yet, but so far it looks very likely that we'll settle on the >> rule that exceptions cann

[PATCH] D108905: [ItaniumCXXABI] Make __cxa_end_catch calls unconditionally nounwind

2023-10-18 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D108905#4654411 , @MaskRay wrote: > In D108905#4654410 , @smeenai wrote: > >> In D108905#4654403 , @ChuanqiXu >> wrote: >> >>> In D108905#46

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

2023-10-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. LGTM since this is exactly what we do in the downstream. The effects of the change in our workloads is 4% size reduction in a coroutine intensive project. (But it requires the coroutine's final suspend can't except via symetric transfer. I was meant to send a paper to

[PATCH] D108905: [ItaniumCXXABI] Add -fassume-nothrow-exception-dtor to assume that an exception object' destructor is nothrow

2023-10-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. LGTM since this is exactly what we do in the downstream. The effects of the change in our workloads is 4% size reduction in a coroutine intensive project. (But it requires the coroutine's final suspend can't except via symetric transfer. I was meant to send a paper to

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D91245#2437518 , @MyDeveloperDay wrote: > What does `the behavior goes wrong` mean? why can't it be left aligned? Because when we use PointerAlignment attribute, I think we mean that the pointer need to be aligned in declar

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 311485. ChuanqiXu added a comment. Re-upload patch with context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91245/new/ https://reviews.llvm.org/D91245 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp Index:

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D91245#2437741 , @Quuxplusone wrote: > Peanut gallery says: It seems to me that your root problem here is that > `co_yield` is not being recognized by the parser as a keyword, and so you're > seeing e.g. `co_yield* p;` the s

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 311789. ChuanqiXu added a comment. add `verifyFormat("return *a", PASLeftStyle);` to clarify the change CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91245/new/ https://reviews.llvm.org/D91245 Files: clang/lib/Format/TokenAnnotator.cpp clang/

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7759-7761 + // The default setting for PointerAlignment is PAS_Right. + // But if we set PointerAlignment as PAS_Left, the formatter + // would mis-format the pointer alignment.

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 311849. ChuanqiXu added a comment. Edit the comment to avoid misleading. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91245/new/ https://reviews.llvm.org/D91245 Files: clang/lib/Format/TokenAnnotator.cpp clang/unittests/Format/FormatTest.cpp

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/unittests/Format/FormatTest.cpp:7759-7761 + // The default setting for PointerAlignment is PAS_Right. + // But if we set PointerAlignment as PAS_Left, the formatter + // would mis-format the pointer alignment.

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D91245#2454504 , @MyDeveloperDay wrote: > Aside: Would you be prepared to take a look at D34225: [clang-format] Teach > clang-format how to handle C++ coroutines > which is pretty much been

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D91245#2454610 , @MyDeveloperDay wrote: > I don't like seeing users having to use ```// clang-format off``` > > auto try_sequence = [](int& ref) -> return_ignore { > /* clang-format off */ > for

[PATCH] D91245: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid misleading pointer alignment

2020-12-15 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8b48d2437320: [clang-format] Recognize c++ coroutine keywords as unary operator to avoid… (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.o

[PATCH] D91243: [NFC][Coroutines] Remove unused `IsImplicit` argument in maybeTailCall

2020-11-11 Thread Chuanqi Xu via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGcd89c4dbdd3a: [NFC][coroutines] remove unused argument in SemaCoroutine (authored by ChuanqiXu). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monore

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; Can't we merge these? ==

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; ychen wrote: > ChuanqiXu wrote: >

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-05-12 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/AST/StmtCXX.h:356-359 Expr *Allocate = nullptr; Expr *Deallocate = nullptr; +Expr *AlignedAllocate = nullptr; +Expr *AlignedDeallocate = nullptr; ychen wrote: > ChuanqiXu wrote: >

[PATCH] D102465: [Coroutines] Mark every parameter

2021-05-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. If it would take times to mark parameter passed by value only, I think we could proceed with this one first. We can try to optimize it later. After all, correctness is most important. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-21 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @ldionne @Quuxplusone Now the compiler wouldn't emit warning for any more. So I think this is good. Do you feel good with this? This is necessary to conform the implementation of coroutine since the compiler would search coroutine components in corresponding namespac

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-23 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D108696#3018045 , @ldionne wrote: > This sounds more reasonable to me, however we need to ship `` in > libc++ before we enable this, or else we're going to start suggesting that > users include `` when we don't have it. We

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-09-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @ldionne @Quuxplusone gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108696/new/ https://reviews.llvm.org/D108696 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2819871 , @ychen wrote: > - Merge D102145 by @ChuanqiXu's request. Thanks a lot. Could you add me as a reviewer? So that I can see this patch in my main page. Repository: rG LLVM

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-15 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. It looks not easy to review completely. After a rough review, the first suggest is to remove the contents that should be in 'D102147 ', it makes the complex problem more complex I think. I would try to do more detailed review and test

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-16 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. It misses the part in llvm now. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:437-475 +void emitDynamicAlignedDealloc(CodeGenFunction &CGF, + llvm::BasicBlock *AlignedFreeBB, + llvm::CallInst

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-20 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. A remained question. - what's the semantics if user specified their allocation/deallocation functions? Previously, we discussed for the ::operator new and ::operator delete. But what would we do for user specified allocation/deallocation functions? It looks like we w

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-22 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2832446 , @ychen wrote: > In D97915#2829581 , @ChuanqiXu wrote: > >> A remained question. >> >> - what's the semantics if user specified their allocation/deallocation >> functio

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-06-24 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2835147 , @ychen wrote: > In D97915#2832667 , @ChuanqiXu wrote: > >> In D97915#2832446 , @ychen wrote: >> >>> In D97915#2829581

[PATCH] D105877: [Coroutines] Run coroutine passes by default

2021-08-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D105877#2923257 , @nikic wrote: > I noticed that this change had a measurable impact on `O0` memory usage, > which I wouldn't have expected > (https://llvm-compile-time-tracker.com/compare.php?from=0f9e6451a836886f39137818c

[PATCH] D105877: [Coroutines] Run coroutine passes by default

2021-08-05 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D105877#2924157 , @ChuanqiXu wrote: > In D105877#2923257 , @nikic wrote: > >> I noticed that this change had a measurable impact on `O0` memory usage, >> which I wouldn't have expect

[PATCH] D105066: [Coroutines] Remove CoroElide from O0 pipeline

2021-06-28 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. On O0, it is possible to inline if the user marked the function with `always_inline`. Since CoroElide is kind of optimization, it should be OK to skip in O0. Out of curiosity, what's the reason that you want to remove it? Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D105066: [Coroutines] Remove CoroElide from O0 pipeline

2021-06-28 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D105066#2845978 , @lxfind wrote: > Coroutine functions cannot be inlined before splitting, even if it's marked > "always_inline" Yeah, but it may be inlined after splitting, which could trigger coro elide. > in fact, we s

[PATCH] D105066: [Coroutines] Remove CoroElide from O0 pipeline

2021-06-28 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. > That's a separate topic though. Let's agree on this diff first and then I can > explain more about the always_inline issue. Yeah, maybe we should discuss it on the llvm-dev thread. R

[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. > note that we don't really need to run Inliner again on the ramp function > after split This isn't accurate. The inline may run again for ramp function after split and it's required by coro elide. It seems like that we don't need the attribute `CORO_PRESPLIT_ATTR` a

[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. In D95807#2847449 , @lxfind wrote: > In D95807#2846358 , @ChuanqiXu wrote: > >>> note that we don't reall

[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D95807#2849120 , @lxfind wrote: > In D95807#2849053 , @aeubanks wrote: > >> this will run the function simplification pipeline twice on every single >> function when coroutines are ena

[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D95807#2849128 , @lxfind wrote: >> If coroutine ramp function couldn't get inlined, it would disable coroutine >> elide optimization. Could you elaborate more on why do you want to do that? > > Ramp function will eventually b

[PATCH] D95807: [Coroutines] Add the newly generated SCCs back to the CGSCC work queue after CoroSplit actually happened

2021-06-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: llvm/lib/Transforms/Coroutines/CoroSplit.cpp:2112-2114 StringRef Value = Attr.getValueAsString(); LLVM_DEBUG(dbgs() << "CoroSplit: Processing coroutine '" << F.getName() << "' state: " << Value << "\n")

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I am a little busy this week. I would try to look into this next week if possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97915/new/ https://reviews.llvm.org/D97915 ___

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2848816 , @ychen wrote: >> Thanks for clarifying. Let's solve the semantics problem first. >> With the introduction about 'raw frame', I think it's necessary to introduce >> this concept in the section 'Switched-Resume

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2860916 , @ychen wrote: > In D97915#2859237 , @ChuanqiXu wrote: > >> In D97915#2848816 , @ychen wrote: >> Thanks for clarifying. Let

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-06 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2861036 , @ychen wrote: > In D97915#2860984 , @ChuanqiXu wrote: > >> In D97915#2860916 , @ychen wrote: >> >>> In D97915#2859237

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2862419 , @ychen wrote: >> It looks not hard to implement. And we don't need to refactor the CodeGen >> part a lot. In this way, I think the main effort to support `::operator >> new(size_t, align_t)` would be in the

[PATCH] D97915: [Coroutines] Handle overaligned frame allocation

2021-07-07 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D97915#2863615 , @ychen wrote: > That's just my conclusion based on @rjmccall's suggestion > (https://reviews.llvm.org/D100739#2717582) and my following responses. I guess you got the conclusion from this: > 2d. Use the cor

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-10-25 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D108696#3086011 , @lewissbaker wrote: > So am I correct in understanding that the main issue with the chicken/egg > problem for updating both the compiler to use the new stdlib facilities and > updating the stdlib faciliti

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-10-28 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rsmith gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110215/new/ https://reviews.llvm.org/D110215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-10-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked 5 inline comments as done. ChuanqiXu added a comment. In D108696#3090484 , @Quuxplusone wrote: > @lewissbaker wrote: > >> #include // which transitively includes >> #include > > Good example! I had not previously been thinking abo

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-10-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 383376. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110215/new/ https://reviews.llvm.org/D110215 Files: clang/include/clang/Basic/Module.h clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDec

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-10-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked 7 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:722-723 +void Sema::PopGlobalModuleFragment() { + assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() && + "left the wrong module scope,

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-10-29 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D108696#3096822 , @Quuxplusone wrote: > In D108696#3095789 , @ChuanqiXu > wrote: > >> Since there are other `experimental/*` headers moved in to normal include >> paths, I guess th

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a type out of a module

2021-10-31 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: rsmith, aaron.ballman. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This fixes https://bugs.llvm.org/show_bug.cgi?id=47116. According to https://eel.is/c++draft/mo

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a type out of a module

2021-11-01 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:5748-5750 +else if (isa(Cur)) + Diag(Loc, diag::err_invalid_declarator_in_export) + << Name << cast(DC) << SS.getRange(); aaron.ballman wrote: > I don't believe this is suf

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 384645. ChuanqiXu retitled this revision from "[C++20] [Module] Fix front end crashes when trying to export a type out of a module " to "[C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared". ChuanqiXu ad

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7784-7785 "because namespace %1 does not enclose namespace %2">; +def err_invalid_declarator_in_export : Error<"cannot export %0 here " + "because it had be declared in %1.">; def e

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:5748-5750 +else if (isa(Cur)) + Diag(Loc, diag::err_invalid_declarator_in_export) + << Name << cast(DC) << SS.getRange(); ChuanqiXu wrote: > aaron.ballman wrote: > > Chuanq

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

2021-11-03 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @lxfind @Quuxplusone @lewissbaker Thanks for looking into this! I have committed this. Since the remained problem is that whether or not should we support the case that user uses "std::coroutine*" and "std::experimental::coroutine*" at the same time. As the reason we s

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-11-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rsmith @aaron.ballman gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110215/new/ https://reviews.llvm.org/D110215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/m

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu planned changes to this revision. ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7784-7785 "because namespace %1 does not enclose namespace %2">; +def err_invalid_declarator_in_export : Error<"cannot export %0 here " +

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu requested review of this revision. ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7784-7785 "because namespace %1 does not enclose namespace %2">; +def err_invalid_declarator_in_export : Error<"cannot export %0 here " +

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-04 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 384968. ChuanqiXu added a comment. Updated diagnostic messages. It looks good to me to reject for exporting non-namespace-scope names in `diagnoseQualifiedDeclaration()`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112903/new/ https://reviews.l

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 385440. ChuanqiXu added reviewers: aaron.ballman, rsmith, urnathan, dblaikie. ChuanqiXu set the repository for this revision to rG LLVM Github Monorepo. ChuanqiXu added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Mo

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D113391#3115080 , @tschuett wrote: > This sounds like an enum. > > enum class ModuleKind { > C++20, > Clang, > Unsupported > } In fact, there is another value `ModuleTS`. But it looks not bad. And the variable `CP

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 385680. ChuanqiXu added a comment. Herald added subscribers: carlosgalvezp, kbarton, nemanjai. Herald added a project: clang-tools-extra. Update part in clang-tools-extra Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D113391#3115410 , @tschuett wrote: > It would enforce that there is exactly one module mode live. Yeah. But my consideration is that it would enforce another variable and it couldn't eliminate the two existing variables. So

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-08 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3445-3447 - // -fmodules enables the use of precompiled modules (off by default). - // Users can pass -fno-cxx-modules to turn off modules support for - // C++/Objective-C++ programs. --

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:1 -[ - { -"directory": "DIR", -"command": "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fcxx-modules -

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added inline comments. Comment at: clang/test/ClangScanDeps/Inputs/header-search-pruning/cdb.json:1 -[ - { -"directory": "DIR", -"command": "clang -E DIR/header-search-pruning.cpp -Ibegin -I1 -Ia -I3 -I4 -I5 -I6 -Ib -I8 -Iend DEFINES -fmodules -fcxx-modules -

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 385765. ChuanqiXu added a reviewer: jansvoboda11. ChuanqiXu added a comment. Undo unnecessary style change in *.json files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113391/new/ https://reviews.llvm.org/D1

[PATCH] D113391: [Modules] Don't support clang module and c++20 module at the same time

2021-11-09 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu abandoned this revision. ChuanqiXu added a comment. Oh, OK. If this is not the direction the community want, we might not go on this direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113391/new/ https://reviews.llvm.org/D113391 _

[PATCH] D113545: [C++20] [Module] Support reachable definition initially/partially

2021-11-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu created this revision. ChuanqiXu added reviewers: rsmith, aaron.ballman, erichkeane, urnathan, hubert.reinterpretcast. ChuanqiXu added a project: clang. Herald added a subscriber: dexonsmith. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This fixes

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

2021-11-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @rsmith ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D110215/new/ https://reviews.llvm.org/D110215 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-co

[PATCH] D112903: [C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared

2021-11-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. @aaron.ballman @urnathan ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112903/new/ https://reviews.llvm.org/D112903 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D112903: [C++20] [Module] Fix bug47716 and implement [module.interface]/p6

2021-11-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu updated this revision to Diff 386728. ChuanqiXu retitled this revision from "[C++20] [Module] Fix front end crashes when trying to export a qualified entity which is already declared" to "[C++20] [Module] Fix bug47716 and implement [module.interface]/p6". ChuanqiXu edited the summary of

[PATCH] D112903: [C++20] [Module] Fix bug47716 and implement [module.interface]/p6

2021-11-11 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu marked 3 inline comments as done. ChuanqiXu added inline comments. Comment at: clang/test/CXX/module/module.interface/p2-2.cpp:1 +// The intention of this file to check we could only export declarations in namesapce scope. +// I move them here since th

<    1   2   3   4   5   6   7   8   9   10   >