[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale abandoned this revision. oydale added a comment. compnerd added a triple to the test case and re-committed the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 ___

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

jfb reopened this revision. jfb added a comment. This revision is now accepted and ready to land. Reverted in r367051, it's causing failures: http://lab.llvm.org:8011/builders/clang-cmake-armv8-full/builds/13658/steps/ninja%20check%201/logs/FAIL%3A%20Clang%3A%3Apr40771-ctad-with-lambda-copy-captu

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

compnerd closed this revision. compnerd added a comment. SVN r367042 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale added a comment. @rsmith, @rjmccall, I don't have access to commit this myself, could one of you commit it? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale accepted this revision. oydale added a comment. Feel free to commit this patch on my behalf. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 ___ cfe-comm

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209701. oydale added a comment. Changed test to use CHECK-NEXT, -discard-value-names, and pattern matching Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri resigned from this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. In D64656#1584437 , @oydale wrote: > My understanding of the issue is that clang emits incorrect IR. Without my > fix and when disabling t

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale added a comment. My understanding of the issue is that clang emits incorrect IR. Without my fix and when disabling the assertion mentioned in the commit message by commenting it out, llvm-lit gives the following output when executed against the minimal test case in the current version of

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri added a comment. In D64656#1584424 , @lebedev.ri wrote: > Hm, i have a question about this fix. > As it can be seen the C++17 code is successfully codegened by clang to LLVM > IR, and the actual failure is in LLVM middle-end optimization pass:

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri added a comment. Hm, i have a question about this fix. As it can be seen the C++17 code is successfully codegened by clang to LLVM IR, and the actual failure is in LLVM middle-end optimization pass: https://godbolt.org/z/P3RB23 1. Please file a bug about that pass crash, include that

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale added a comment. I have added my initial attempt at verifying the cleanup code generated in this scenario, it's probably going to require modifications. The original IR can be found at https://reviews.llvm.org/P8156. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209690. oydale added a comment. Initial attempt at IR output checks Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang/lib/CodeGen/CGExprAgg.cpp clang/test

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Sorry, i still don't like the test. You want to check the produced IR. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209683. oydale added a comment. Replaced -emit-obj with -emit-llvm-only in test case Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang/lib/CodeGen/CGExprAgg

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:1 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions %s +// PR40771: check that this input does not crash or assert This will run the ba

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rjmccall accepted this revision. rjmccall 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/D64656/new/ https://reviews.llvm.org/D64656 ___

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209627. oydale added a comment. Minimized test case further, removed misleading CHECK from comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang/lib/Cod

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale marked an inline comment as done. oydale added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:2 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s +// CHECK no crash + rjmccall wrote

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rjmccall added a comment. Oh, I see you've already answered that. I think it's fine to just leave this testing debug output if generated optimized output doesn't affect it; the bulk of our regression testing is with assertions-enabled compilers anyway. Repository: rG LLVM Github Monorepo C

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rjmccall added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:2 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s +// CHECK no crash + oydale wrote: > lebedev.ri wrote: > > Either add miss

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale marked 2 inline comments as done. oydale added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:1 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s +// CHECK no crash lebedev.ri wrote

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:1 +// RUN: %clang_cc1 -emit-obj --std=c++17 -fcxx-exceptions -fexceptions -O1 %s +// CHECK no crash Why do you need `-O1`? Comment at

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rsmith added inline comments. Comment at: clang/test/CodeGenCXX/pr40771-ctad-with-lambda-copy-capture.cpp:4-24 +struct outer +{ +struct inner +{ +~inner() {} +}; + Please use a more minimal testcase, such as: ``` struct P { ~P(); }; struct Q

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209585. oydale added a comment. Reduce test case to a single execution with -O1 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang/lib/CodeGen/CGExprAgg.cpp

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale updated this revision to Diff 209584. oydale added a comment. Added regression test based on code in bug report Moved logic to a lambda Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 Files: clang/

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

rjmccall added inline comments. Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1627 +llvm::Constant::getNullValue(CGF.Int8PtrTy), +CharUnits::One()); // placeholder + Please declare a lambda to add a cleanup which implicitly creates this dominator a

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

lebedev.ri added a comment. Test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64656/new/ https://reviews.llvm.org/D64656 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D64656: Ensure placeholder instruction for cleanup is created

oydale created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. A placeholder instruction for use in generation of cleanup code for an initializer list would not be emitted if the base class contained a non-trivial destructor and the class contains no fields of