[PATCH] D117603: [clang] Don't typo-fix an expression in a SFINAE context

ChuanqiXu added a comment. The problem looks similar with what we met in: https://github.com/llvm/llvm-project/issues/52909#issuecomment-1021631943 Maybe both of the problem could be solved at the same time. > Anyone see how this patch might have caused a placeholder type to survive > into mang

[PATCH] D118437: [NFC] [Modules] Refactor ODR checking for default template argument in ASTReader

ChuanqiXu created this revision. ChuanqiXu added reviewers: urnathan, erichkeane, aaron.ballman, rjmccall, rsmith. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This patch tries to refactor ODR checking process for default t

[PATCH] D118518: [clang][NFC] Change some ->getType()->isPlaceholderType() to just ->hasPlaceholderType()

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118518/new/ https://reviews.llvm.org/D118518

[PATCH] D120084: [clang][DOC] Document module mangler changes

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120084/new/ https://reviews.llvm.org/D120084 ___ cfe-commits mailing list cfe-co

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

ChuanqiXu added a comment. Thanks for explanation. Now it looks good to me. Let's accept it formally after the series of partition landed and so that we could add test about partitions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:177 + if (IsPartition) { +ModuleName += ":"; +ModuleName += stringFromPath(Partition); iains wrote: > urnathan wrote: > > iains wrote: > > > ChuanqiXu wrote: > > > > I chose '-' i

[PATCH] D118598: [C++20][Modules][7/8] Find the primary interface name for a module.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/Module.h:527 +} +return StringRef(Name); + } Here we could return `Name` simply. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118598/

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

ChuanqiXu updated this revision to Diff 409872. ChuanqiXu retitled this revision from "[C++20] [Modules] Remain internal-linkage variables in module interface unit" to "[C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit ". ChuanqiXu edited the summa

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

ChuanqiXu added inline comments. Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global -// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global +// CHECK-DAG: @inline_var_

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682 +Expr::EvalResult EVRX, EVRY; +if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) || +!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C)) + return false; + +APValue

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu accepted this revision. ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:177 + if (IsPartition) { +ModuleName += ":"; +ModuleName += stringFromPath(Partition); urnathan wrote: > iains wrote: > > ChuanqiXu wrote: > > >

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

ChuanqiXu added a comment. In D119409#3332313 , @dblaikie wrote: > (maybe relevant: For what it's worth: I originally implemented inline > function homing in modules codegen for Clang Header Modules - the results I > got for object file size in an -O0 b

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

ChuanqiXu updated this revision to Diff 410270. ChuanqiXu edited the summary of this revision. ChuanqiXu added a reviewer: iains. ChuanqiXu added a comment. Implement reachable definition initially/partially. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113545/new/ https://reviews.llvm

[PATCH] D113972: [RFC] [C++20] [Module] Support module partitions initially

ChuanqiXu abandoned this revision. ChuanqiXu added a subscriber: iains. ChuanqiXu added a comment. Now we prefer @iains's patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113972/new/ https://reviews.llvm.org/D113972 __

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. I think it is helpful to add some tests from https://reviews.llvm.org/D113972 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118586/new/ https://reviews.llvm.org/D118586 ___ cfe

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. In D118586#3335191 , @iains wrote: > In D118586#3335167 , @ChuanqiXu > wrote: > >> I think it is helpful to add some tests from https://reviews.llvm.org/D113972 > > Actually, in respons

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. (May off the patch) BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between c

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. In D118586#3336865 , @iains wrote: > In D118586#3336612 , @ChuanqiXu > wrote: > >> (May off the patch) > > I am not sure what you mean here. I mean this comment is not related to the p

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

ChuanqiXu added a comment. In D118586#3336933 , @iains wrote: > In D118586#3336892 , @ChuanqiXu > wrote: > >> In D118586#3336865 , @iains wrote: >> >>> In D118586#3336612

[PATCH] D120352: [C++20][Modules] Rework testcase to use split file [NFC].

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120352/new/ https://reviews.llvm.org/D120352

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu created this revision. ChuanqiXu added reviewers: urnathan, iains, rsmith, aaron.ballman, erichkeane. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. Before the patch, the compiler would crash for the test due to inco

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

ChuanqiXu added a comment. @urnathan FWIW, D114714 is landed~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352 ___ cfe-commits mailing list cfe-commits@lists.llvm.or

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu created this revision. ChuanqiXu added reviewers: jansvoboda11, sammccall, urnathan, Quuxplusone. ChuanqiXu added a project: clang. Herald added a subscriber: dang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This patch allows user to use C++20 mo

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu updated this revision to Diff 411996. ChuanqiXu added a comment. Add test in CodeGen. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120397/new/ https://reviews.llvm.org/D120397 Files: clang/lib/AST/Decl.cpp clang/test/CodeGenCXX/inconsistent-export-template.cpp clang/te

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu added inline comments. Comment at: clang/test/CodeGenCXX/inconsistent-export-template.cpp:8 + +// CHECK: void @_Z1fIiEvv +template <> The mangled name should contain module name if D118352 is ready. Comment at: clang/test/Modules/inco

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23 +// FIXME: We should reject following specialization, +// since it tries to export a name which is already introduced. +export template <> +void f1() { + iains wro

[PATCH] D120764: [C++20][Modules] Improve efficiency of isModuleParition method.

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120764/new/ https://reviews.llvm.org/D120764

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

ChuanqiXu added a comment. Herald added a project: All. @rsmith gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/D118034 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

ChuanqiXu added a comment. Herald added a project: All. ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119409/new/ https://reviews.llvm.org/D119409 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/ma

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

ChuanqiXu added a comment. Herald added a project: All. 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/ma

[PATCH] D120793: [NFC] [C++20] [Modules] Simplify ActOnModuleImport by merging Path and Parition

ChuanqiXu created this revision. ChuanqiXu added reviewers: iains, urnathan. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. The parameter of `Path` and `Partition` of ActOnModuleImport could be mer

[PATCH] D120793: [NFC] [C++20] [Modules] Simplify ActOnModuleImport by merging Path and Parition

ChuanqiXu updated this revision to Diff 412404. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120793/new/ https://reviews.llvm.org/D120793 Files: clang/include/clang/Sema/Sema.h clang/lib/Parse/Parser.cpp clang/lib/Sema/SemaModule.cpp

[PATCH] D120793: [NFC] [C++20] [Modules] Simplify ActOnModuleImport by merging Path and Parition

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:415 + return ActOnModuleImport(StartLoc, ExportLoc, ImportLoc, Mod, Path); } iains wrote: > it looks like there is an extra blank line before the closing brace.. No, it isn't. It shows

[PATCH] D120793: [NFC] [C++20] [Modules] Simplify ActOnModuleImport by merging Path and Parition

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3eb2da76d770: [NFC] [C++20] [Modules] Simplify ActOnModuleImport by merging Path and Parition (authored by ChuanqiXu). Repository: rG LLVM Github

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu created this revision. ChuanqiXu added reviewers: urnathan, iains. ChuanqiXu added a project: clang. Herald added a project: All. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. It is simpler to search for module unit by `-fprebuilt-module-path` optio

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added a subscriber: MyDeveloperDay. ChuanqiXu added inline comments. Comment at: clang/test/Modules/search-partitions.cppm:20 +//--- partition1.cpp +export module A : Part1; + @MyDeveloperDay hi, I remember the support for partitions in clang-format is

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

ChuanqiXu updated this revision to Diff 412657. ChuanqiXu added a comment. Rebase and update tests. 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/Module.h clang/includ

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu updated this revision to Diff 412886. ChuanqiXu added a comment. Address comments: - File an issue and add a reference to it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120397/new/ https://reviews.llvm.org/D120397 Files: clang/lib/AST/Decl.cpp clang/test/CodeGenCXX/in

[PATCH] D120397: [C++20] [Modules] Make the linkage consistent for template and its specialization

ChuanqiXu added inline comments. Comment at: clang/test/Modules/inconsist-export-template.cpp:19-23 +// FIXME: We should reject following specialization, +// since it tries to export a name which is already introduced. +export template <> +void f1() { + dblaikie

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 412899. ChuanqiXu added a comment. Format tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/search-partitions.cpp Index: clang/test/Mod

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu added inline comments. Comment at: clang/test/Modules/search-partitions.cppm:20 +//--- partition1.cpp +export module A : Part1; + iains wrote: > ChuanqiXu wrote: > > @MyDeveloperDay hi, I remember the support for partitions in clang-format > > is done

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

ChuanqiXu added a comment. If I don't misread the codes, it looks like `mangleModuleInitializer` is not called. --- Now we could add test for partitions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118352/new/ https://reviews.llvm.org/D118352 __

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu updated this revision to Diff 412928. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 Files: clang/lib/Lex/HeaderSearch.cpp clang/test/Modules/search-partitions.cpp Index: clang/test

[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. In D120874#3358870 , @tschuett wrote: > Stupid question: this works with Windows as well? And the `BTW` sounds odd. > gcc also decided to use a dash as the separator. It should work

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. In D120540#3359394 , @iains wrote: > 1. I agree 100% that the driver needs to be able to process in "c++20 modules > mode"; Yeah, not all the projects could be able to run in c++20 mode. We use `-std=c++17` + `-fcoroutines-ts

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

ChuanqiXu added a comment. In D118352#3359626 , @urnathan wrote: > In D118352#3358864 , @ChuanqiXu > wrote: > >> If I don't misread the codes, it looks like `mangleModuleInitializer` is not >> called. > > > >> N

[PATCH] D120540: [Driver] Enable to use C++20 modules standalone by -fcxx-modules

ChuanqiXu added a comment. In D120540#3359454 , @iains wrote: > In D120540#3359446 , @ChuanqiXu > wrote: > >> In D120540#3359394 , @iains wrote: >> >>> 1. I agree 100% th

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

ChuanqiXu added a comment. @urnathan It looks @rsmith is not available this time. Would you like to accept this one? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118034/new/ https://reviews.llvm.org/D118034 ___ cfe-commits mailing list cfe-

[PATCH] D121095: [C++20][Modules][HU 1/5] Introduce header units as a module type.

ChuanqiXu added inline comments. Comment at: clang/include/clang/Sema/Sema.h:2978-2980 + /// The parser has begun a translation unit to be compiled as a C++20 + /// Header Unit. + void ActOnStartOfHeaderUnit(); From the implementation, I think it should be cal

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

ChuanqiXu accepted this revision. ChuanqiXu added a comment. This revision is now accepted and ready to land. LGTM, then. BTW, I see there is already no dependency with D119833 in code. But it shows dependency still in phab. CHANGES SINCE LAST ACTION https:

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

ChuanqiXu added a comment. > This is an alternative to D97915 which > missed proper deallocation of the over-allocated frame. This patch handles > both allocations and deallocations. If D97915 is not needed, we should abandon i

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

ChuanqiXu added a comment. In D100739#2713579 , @ychen wrote: > In D100739#2711698 , @ChuanqiXu > wrote: > >>> This is an alternative to D97915 which >>> missed proper deallocat

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

ChuanqiXu added a comment. In D100739#2717582 , @rjmccall wrote: > That's not really what I meant, no. I meant it would be better to find a way > to use an allocator that promises to return a well-aligned value when > possible. We've talked about this b

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

ChuanqiXu added a comment. In D100739#2718514 , @ychen wrote: > Oh, right now C++ coroutine standard is written in the way that the aligned > new is not searched by the frontend. The limitation will be lifted in C++23 > (hopefully). I see. I am curious

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

ChuanqiXu added a comment. May I ask a question may be too simple? What if the user specify the alignment for promise (or any other local variables) to 128 or even 256? Since it looks like all the discuss before assumes that the largest alignment requirement is 64. Repository: rG LLVM Githu

[PATCH] D100739: [Coroutines] Handle overaligned frame allocation (2)

ChuanqiXu added a comment. In D100739#2727808 , @ychen wrote: > In D100739#2718528 , @ChuanqiXu > wrote: > >> In D100739#2718514 , @ychen wrote: >> >>> Oh, right now C++

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

ChuanqiXu added a comment. In D97915#2727787 , @ychen wrote: > In D97915#2727759 , @ChuanqiXu wrote: > >> May I ask a question may be too simple? What if the user specify the >> alignment for promise (or any other

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

ChuanqiXu added a comment. This code snippets confused me before: coro.alloc.align: ; preds = %coro.check.align %mask = sub i64 %11, 1 %intptr = ptrtoint i8* %call to i64 %over_boundary = add i64 %intptr, %mask %inverted_mask = xor i64 %mask, -1

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

ChuanqiXu added a comment. > which should be correct. It is implemented by > CodeGenFunction::EmitBuiltinAlignTo. I agree it is correct. I just want to say we should comment it to avoid confusing. Since the patch could handle the case if the frontend tries to search `::operator new(size_t, al

[PATCH] D102147: [Clang][Coroutines] Implement P2014R0 Option 1 behind -fcoroutines-aligned-alloc

ChuanqiXu added a comment. Since D97915 would fix the problem that the variables in the frame may not be aligned, I think this option `fcoroutines-aligned-alloc` won't affect normal programmers other than language lawyers. Do you think so? Repository: rG LL

[PATCH] D102147: [Clang][Coroutines] Implement P2014R0 Option 1 behind -fcoroutines-aligned-alloc

ChuanqiXu added a comment. In D102147#2747939 , @ychen wrote: > In D102147#2747611 , @ChuanqiXu > wrote: > >> Since D97915 would fix the problem that >> the variables in the fra

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu marked an inline comment as done. ChuanqiXu added inline comments. Comment at: llvm/docs/CommandGuide/llvm-objcopy.rst:539 -To report bugs, please visit . +To report bugs, please visit . -

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu updated this revision to Diff 396737. ChuanqiXu marked an inline comment as done. ChuanqiXu added a comment. Drop the change in lld. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116351/new/ https://reviews.llvm.org/D116351 Files: clang-tools-extra/docs/clang-doc.rst clan

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu added inline comments. Comment at: llvm/utils/gn/secondary/clang/include/clang/Config/BUILD.gn:10 output = "$target_gen_dir/config.h" values = [ +"BUG_REPORT_URL=https://github.com/llvm/llvm-project/issues/";, xbolva00 wrote: > This url should

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

ChuanqiXu added a comment. @Quuxplusone do you feel good with the current message? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115867/new/ https://reviews.llvm.org/D115867 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://list

[PATCH] D115610: [C++20] [Modules] Don't create multiple global module fragment

ChuanqiXu added a comment. @rsmith gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115610/new/ https://reviews.llvm.org/D115610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

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

ChuanqiXu added a comment. @aaron.ballman @urnathan gentle 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

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

ChuanqiXu added a comment. @rjmccall gentle ping~ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115790/new/ https://reviews.llvm.org/D115790 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listin

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

ChuanqiXu updated this revision to Diff 397217. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115790/new/ https://reviews.llvm.org/D115790 Files: clang/lib/CodeGen/CGCoroutine.cpp clang/test/CodeGenCoroutines/coro-always-inline.cpp cla

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

ChuanqiXu marked 2 inline comments as done. ChuanqiXu added inline comments. Comment at: llvm/lib/Transforms/Coroutines/CoroEarly.cpp:198 case Intrinsic::coro_id_async: F.addFnAttr(CORO_PRESPLIT_ATTR, PREPARED_FOR_SPLIT); break; rjmccall

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

ChuanqiXu added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:0-3 +def warn_always_inline_coroutine : Warning< + "A coroutine marked always_inline might not be inlined properly." + >, + InGroup; Quuxplusone wrote: > Chua

[PATCH] D100282: [Coroutines] Set presplit attribute in Clang instead of CoroEarly pass

ChuanqiXu closed this revision. ChuanqiXu added a comment. This should be covered in: https://reviews.llvm.org/D115790 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100282/new/ https://reviews.llvm.org/D100282 _

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu added a comment. @asl hi, do you feel good with this revision? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116351/new/ https://reviews.llvm.org/D116351 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc75cedc237f9: [Coroutines] Set presplit attribute in Clang and mlir (authored by ChuanqiXu). Changed prior to commit: https://reviews.llvm.org/D11

[PATCH] D115790: [Coroutines] Set presplit attribute in Clang and mlir

ChuanqiXu added a comment. In D115790#3221307 , @MaskRay wrote: > Early heads-up: we see a failure with > `llvm/lib/Transforms/Coroutines/CoroEarly.cpp:186 in bool (anonymous > namespace)::Lowerer::lowerEarlyIntrinsics(llvm::Function &): > F.hasFnAttri

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu updated this revision to Diff 397763. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116351/new/ https://reviews.llvm.org/D116351 Files: clang-tools-extra/docs/clang-doc.rst clang/docs/CommandGuide/clang.rst clang/www/c_status

[PATCH] D116351: Update Bug report URL to Github Issues

ChuanqiXu added a comment. It looks like @asl isn't here and many experienced guys accepted this. So I think it might should be good to commit this one. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116351/new/ https://reviews.llvm.org/D116351 _

[PATCH] D116351: Update Bug report URL to Github Issues

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGbbce75e352be: Update Bug report URL to Github Issues (authored by

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

ChuanqiXu updated this revision to Diff 397835. ChuanqiXu added a comment. Herald added a reviewer: aaron.ballman. Address comments: - Update warning message as Mathias's suggestion. - Update document. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115867/new/ https://reviews.llvm.org/D

[PATCH] D116792: [AST] lookup in parent DeclContext for transparent DeclContext

ChuanqiXu created this revision. ChuanqiXu added reviewers: erichkeane, rsmith, rjmccall, aaron.ballman, urnathan. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. The compiler would crash if we lookup for name in transparent d

[PATCH] D116911: [AST] Don't consider 'ExportDecl' when calculating DeclContext 'Encloses'

ChuanqiXu created this revision. ChuanqiXu added reviewers: erichkeane, rjmccall. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This mimics the style of https://github.com/llvm/llvm-project/commit/90010c2e1d60c6a9a4a0b30a113

[PATCH] D116792: [AST] lookup in parent DeclContext for transparent DeclContext

ChuanqiXu added a comment. In D116792#3227379 , @erichkeane wrote: > I had to do something similar for this at one point: > https://github.com/llvm/llvm-project/commit/90010c2e1d60c6a9a4a0b30a113d4dae2b7214eb > > I seem to remember hitting this assert,

[PATCH] D116911: [AST] Don't consider 'ExportDecl' when calculating DeclContext 'Encloses'

This revision was automatically updated to reflect the committed changes. Closed by commit rG86c5b870b2e5: [AST] Don't consider 'ExportDecl' when calculating DeclContext 'Encloses' (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D116792: [AST] lookup in parent DeclContext for transparent DeclContext

This revision was automatically updated to reflect the committed changes. Closed by commit rGd9d63fc1088c: [AST] lookup in parent DeclContext for transparent DeclContext (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116792/ne

[PATCH] D117087: [WIP] [C++20] [Coroutines] Implement return value optimization for get_return_object

ChuanqiXu created this revision. ChuanqiXu added a project: clang. ChuanqiXu requested review of this revision. Herald added a subscriber: cfe-commits. This patch tries to implement RVO for coroutine's return object got from `get_return_object`. >From [dcl.fct.def.coroutine]/p7 we could know that

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

ChuanqiXu added a comment. @aaron.ballman @urnathan gentle 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

[PATCH] D115610: [C++20] [Modules] Don't create multiple global module fragment

ChuanqiXu added a comment. @rsmith gentle ping~ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115610/new/ https://reviews.llvm.org/D115610 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://

[PATCH] D117093: [C++20] [Modules] Exit early if export decl is invalid

ChuanqiXu updated this revision to Diff 399526. ChuanqiXu added a comment. Address comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117093/new/ https://reviews.llvm.org/D117093 Files: clang/lib/Sema/SemaModule.cpp clang/test/Modules/export-in-non-modules.cpp Index: clang/t

[PATCH] D117093: [C++20] [Modules] Exit early if export decl is invalid

ChuanqiXu added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:530-531 + CurContext->addDecl(D); + PushDeclContext(S, D); + aaron.ballman wrote: > Am I understanding properly that this moved up here so that the for loop on > line 553 can traverse

[PATCH] D115867: [C++20] [Coroutines] Warning for always_inline coroutine

ChuanqiXu added a comment. Here is my thought: - Should we warn for this situation? Yes, on the on hand, it behaves differently with normal users imaged. A assistant reason would be that GCC would warn for the case too. - Is the warning message good? I think it at least not worse than what GCC

[PATCH] D117087: [C++20] [Coroutines] Implement return value optimization for get_return_object

ChuanqiXu updated this revision to Diff 399578. ChuanqiXu retitled this revision from "[WIP] [C++20] [Coroutines] Implement return value optimization for get_return_object" to "[C++20] [Coroutines] Implement return value optimization for get_return_object". ChuanqiXu edited the summary of this re

[PATCH] D117087: [C++20] [Coroutines] Implement return value optimization for get_return_object

ChuanqiXu added inline comments. Comment at: clang/lib/CodeGen/CGCoroutine.cpp:481-485 - // The gro variable has to outlive coroutine frame and coroutine promise, but, - // it can only be initialized after coroutine promise was created, thus, we - // split its emission in two

[PATCH] D117093: [C++20] [Modules] Exit early if export decl is invalid

This revision was automatically updated to reflect the committed changes. Closed by commit rG4f8916cfdd94: [C++20] [Modules] Exit early if export decl is not valid (authored by ChuanqiXu). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117093/new/ ht

[PATCH] D114908: [clang] Don't call inheritDefaultTemplateArguments() on CXXDeductionGuideDecl's template parameters

ChuanqiXu added a comment. In D114908#3223981 , @rnk wrote: > Friendly ping for a modules CTAD bugfix. Not sure if you met the same problem with me. In our downstream, we did a workaround like: if (Context.getLangOpts().CPlusPlusModules && To

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

ChuanqiXu created this revision. ChuanqiXu added reviewers: rjmccall, lxfind, junparser, ychen, aaron.ballman. ChuanqiXu added projects: clang, LLVM. Herald added a subscriber: hiraditya. ChuanqiXu requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert. [

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

ChuanqiXu created this revision. ChuanqiXu added reviewers: rjmccall, lxfind, junparser, ychen. ChuanqiXu added projects: clang, LLVM. Herald added a subscriber: hiraditya. ChuanqiXu requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert. I found that the

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

ChuanqiXu updated this revision to Diff 392302. ChuanqiXu edited the summary of this revision. ChuanqiXu added a comment. Use `llvm.coro.end(frame, /*InUnwindPath=*/true)` instead of new coroutine intrinsics. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D115219: [C++20] [Coroutines] Mark coroutine done if unhandled_exception throws

ChuanqiXu added a comment. In D115219#3175582 , @rjmccall wrote: > I agree that you shouldn't call `suspend`, but doesn't `coro.end` have the > behavior of marking the coroutine done? Should we just be calling `coro.end` > on this path? @rjmccall grea

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

ChuanqiXu added a comment. In D115222#3175577 , @rjmccall wrote: > I imagine Gor hoped for this optimization to be implemented someday, assuming > it's still allowed by the language specification. Maybe you would be > interested in pursuing that? It s

[PATCH] D115222: [Coroutines] Remove unused coroutine builtin/intrinsics llvm.coro.param (NFC-ish)

ChuanqiXu added a comment. In D115222#3177269 , @rjmccall wrote: > Like a lot of the switched-resume lowering, this intrinsic is extremely tied > to C++ semantics. If C++ doesn't actually allow the optimization anymore, > then I completely agree that w

<    7   8   9   10   11   12   13   14   >