[clang] [clang-format] Fix crashes in AlignArrayOfStructures (PR #72520)
owenca wrote: > > Thanks for trying it on 17.x. We can't backport it then. > > Any idea if there is another workaround or fix that we could do to target > 17.x? 18 is still pretty far off and clang-format for 17 will soon be > included in a lot of downstream tools. Happy to help out fixing it if you > have any pointers. My bad. #72628 was already "fixed" on 18 before this patch. Can you help bisect it? https://github.com/llvm/llvm-project/pull/72520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 5679f55 - [clang-format] Fix crashes in AlignArrayOfStructures (#72520)
Author: Owen Pan Date: 2023-11-17T14:35:30-08:00 New Revision: 5679f5515b8ba83c804ad871452a95565af76e19 URL: https://github.com/llvm/llvm-project/commit/5679f5515b8ba83c804ad871452a95565af76e19 DIFF: https://github.com/llvm/llvm-project/commit/5679f5515b8ba83c804ad871452a95565af76e19.diff LOG: [clang-format] Fix crashes in AlignArrayOfStructures (#72520) Fixed #54815. Fixed #55269. Fixed #55493. Fixed #68431. Added: Modified: clang/lib/Format/WhitespaceManager.cpp clang/lib/Format/WhitespaceManager.h clang/unittests/Format/FormatTest.cpp Removed: diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp index 32d8b97cc8dadb1..3bc6915b8df0a70 100644 --- a/clang/lib/Format/WhitespaceManager.cpp +++ b/clang/lib/Format/WhitespaceManager.cpp @@ -1316,6 +1316,8 @@ void WhitespaceManager::alignArrayInitializersRightJustified( auto Offset = std::distance(Cells.begin(), CellIter); for (const auto *Next = CellIter->NextColumnElement; Next; Next = Next->NextColumnElement) { + if (RowCount >= CellDescs.CellCounts.size()) +break; auto *Start = (Cells.begin() + RowCount * CellDescs.CellCounts[0]); auto *End = Start + Offset; ThisNetWidth = getNetWidth(Start, End, CellDescs.InitialSpaces); @@ -1379,7 +1381,7 @@ void WhitespaceManager::alignArrayInitializersLeftJustified( auto Offset = std::distance(Cells.begin(), CellIter); for (const auto *Next = CellIter->NextColumnElement; Next; Next = Next->NextColumnElement) { - if (RowCount > CellDescs.CellCounts.size()) + if (RowCount >= CellDescs.CellCounts.size()) break; auto *Start = (Cells.begin() + RowCount * CellDescs.CellCounts[0]); auto *End = Start + Offset; diff --git a/clang/lib/Format/WhitespaceManager.h b/clang/lib/Format/WhitespaceManager.h index df7e9add1cd446f..69398fe411502f1 100644 --- a/clang/lib/Format/WhitespaceManager.h +++ b/clang/lib/Format/WhitespaceManager.h @@ -317,7 +317,7 @@ class WhitespaceManager { auto Offset = std::distance(CellStart, CellStop); for (const auto *Next = CellStop->NextColumnElement; Next; Next = Next->NextColumnElement) { - if (RowCount > MaxRowCount) + if (RowCount >= MaxRowCount) break; auto Start = (CellStart + RowCount * CellCount); auto End = Start + Offset; diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index a12a20359c2fad2..b5021f924a80904 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20709,6 +20709,11 @@ TEST_F(FormatTest, CatchExceptionReferenceBinding) { TEST_F(FormatTest, CatchAlignArrayOfStructuresRightAlignment) { auto Style = getLLVMStyle(); Style.AlignArrayOfStructures = FormatStyle::AIAS_Right; + verifyNoCrash("f({\n" +"table({}, table({{\"\", false}}, {}))\n" +"});", +Style); + Style.AlignConsecutiveAssignments.Enabled = true; Style.AlignConsecutiveDeclarations.Enabled = true; verifyFormat("struct test demo[] = {\n" @@ -21140,6 +21145,33 @@ TEST_F(FormatTest, CatchAlignArrayOfStructuresLeftAlignment) { "that really, in any just world, ought to be split over multiple " "lines\"},{-1, 93463, \"world\"},{7, 5, \"!!\"},};", Style); + + Style.ColumnLimit = 25; + verifyNoCrash("Type foo{\n" +"{\n" +"1, // A\n" +"2, // B\n" +"3, // C\n" +"},\n" +"\"hello\",\n" +"};", +Style); + verifyNoCrash("Type object[X][Y] = {\n" +"{{val}, {val}, {val}},\n" +"{{val}, {val}, // some comment\n" +" {val}}\n" +"};", +Style); + + Style.ColumnLimit = 120; + verifyNoCrash( + "T v[] {\n" + "{ A::aaa, " + "A::, 1, 0.0f, " + "\"" + "\" },\n" + "};", + Style); } TEST_F(FormatTest, UnderstandsPragmas) { ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Fix crashes in AlignArrayOfStructures (PR #72520)
https://github.com/owenca closed https://github.com/llvm/llvm-project/pull/72520 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format][NFC] Skip alignArrayInitializers() for 1-row matrices (PR #72166)
https://github.com/owenca edited https://github.com/llvm/llvm-project/pull/72166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 10025d9 - [NFC] Fix CSPGO clang pass manager test (#72681)
Author: Ellis Hoag Date: 2023-11-17T16:55:18-06:00 New Revision: 10025d947c0b6bfc60a8b0c0f2af40585d173485 URL: https://github.com/llvm/llvm-project/commit/10025d947c0b6bfc60a8b0c0f2af40585d173485 DIFF: https://github.com/llvm/llvm-project/commit/10025d947c0b6bfc60a8b0c0f2af40585d173485.diff LOG: [NFC] Fix CSPGO clang pass manager test (#72681) Fix a `CHECK-NOT` line in a cspgo clang test Added: Modified: clang/test/CodeGen/cspgo-instrumentation_thinlto.c Removed: diff --git a/clang/test/CodeGen/cspgo-instrumentation_thinlto.c b/clang/test/CodeGen/cspgo-instrumentation_thinlto.c index bf113ae22ca..f79433827ce1745 100644 --- a/clang/test/CodeGen/cspgo-instrumentation_thinlto.c +++ b/clang/test/CodeGen/cspgo-instrumentation_thinlto.c @@ -21,11 +21,11 @@ // Ensure Pass PGOInstrumentationUsePass is invoked Once in PreLink. // RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/cs.profdata %s -flto=thin -fdebug-pass-manager -emit-llvm-bc -o %t/foo_fe_pm.bc 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE: Running pass: PGOInstrumentationUse -// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-Running pass: PGOInstrumentationUse +// CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-PRE-NOT: Running pass: PGOInstrumentationUse // // RUN: llvm-lto -thinlto -o %t/foo_pm %t/foo_fe_pm.bc // Ensure Pass PGOInstrumentationUSEPass is invoked in PostLink. -// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fdebug-pass-manager -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST -dump-input=always +// RUN: %clang_cc1 -O2 -x ir %t/foo_fe_pm.bc -fthinlto-index=%t/foo_pm.thinlto.bc -fdebug-pass-manager -fprofile-instrument-use-path=%t/cs.profdata -flto=thin -emit-llvm -o - 2>&1 | FileCheck %s -check-prefix=CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST: Running pass: PGOInstrumentationUse // CHECK-CSPGOUSEPASS-INVOKED-INSTR-USE-POST-NOT: Running pass: PGOInstrumentationUse // ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC] Fix CSPGO clang pass manager test (PR #72681)
https://github.com/ellishg closed https://github.com/llvm/llvm-project/pull/72681 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Reorder Atomic builtins to be consistent. (PR #72718)
https://github.com/Logikable created https://github.com/llvm/llvm-project/pull/72718 None >From 37a08f6b2f0d2bd4e20f14798738a0cbc69bc59e Mon Sep 17 00:00:00 2001 From: Sean Luchen Date: Fri, 17 Nov 2023 23:07:28 + Subject: [PATCH] [clang][NFC] Reorder Atomic builtins to be consistent. --- clang/lib/CodeGen/CGAtomic.cpp | 166 - 1 file changed, 83 insertions(+), 83 deletions(-) diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp index f7c597e181b0bd9..6005d5c51c0e1ac 100644 --- a/clang/lib/CodeGen/CGAtomic.cpp +++ b/clang/lib/CodeGen/CGAtomic.cpp @@ -861,10 +861,10 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { case AtomicExpr::AO__opencl_atomic_init: llvm_unreachable("Already handled above with EmitAtomicInit!"); + case AtomicExpr::AO__atomic_load_n: case AtomicExpr::AO__c11_atomic_load: case AtomicExpr::AO__opencl_atomic_load: case AtomicExpr::AO__hip_atomic_load: - case AtomicExpr::AO__atomic_load_n: break; case AtomicExpr::AO__atomic_load: @@ -880,14 +880,14 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { Dest = EmitPointerWithAlignment(E->getVal2()); break; - case AtomicExpr::AO__c11_atomic_compare_exchange_strong: + case AtomicExpr::AO__atomic_compare_exchange: + case AtomicExpr::AO__atomic_compare_exchange_n: case AtomicExpr::AO__c11_atomic_compare_exchange_weak: - case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: + case AtomicExpr::AO__c11_atomic_compare_exchange_strong: + case AtomicExpr::AO__hip_atomic_compare_exchange_weak: case AtomicExpr::AO__hip_atomic_compare_exchange_strong: case AtomicExpr::AO__opencl_atomic_compare_exchange_weak: - case AtomicExpr::AO__hip_atomic_compare_exchange_weak: - case AtomicExpr::AO__atomic_compare_exchange_n: - case AtomicExpr::AO__atomic_compare_exchange: + case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: Val1 = EmitPointerWithAlignment(E->getVal1()); if (E->getOp() == AtomicExpr::AO__atomic_compare_exchange) Val2 = EmitPointerWithAlignment(E->getVal2()); @@ -938,32 +938,32 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { ShouldCastToIntPtrTy = !MemTy->isFloatingType(); [[fallthrough]]; - case AtomicExpr::AO__c11_atomic_store: - case AtomicExpr::AO__c11_atomic_exchange: - case AtomicExpr::AO__opencl_atomic_store: - case AtomicExpr::AO__hip_atomic_store: - case AtomicExpr::AO__opencl_atomic_exchange: - case AtomicExpr::AO__hip_atomic_exchange: + case AtomicExpr::AO__atomic_fetch_and: + case AtomicExpr::AO__atomic_fetch_nand: + case AtomicExpr::AO__atomic_fetch_or: + case AtomicExpr::AO__atomic_fetch_xor: + case AtomicExpr::AO__atomic_and_fetch: + case AtomicExpr::AO__atomic_nand_fetch: + case AtomicExpr::AO__atomic_or_fetch: + case AtomicExpr::AO__atomic_xor_fetch: case AtomicExpr::AO__atomic_store_n: case AtomicExpr::AO__atomic_exchange_n: case AtomicExpr::AO__c11_atomic_fetch_and: + case AtomicExpr::AO__c11_atomic_fetch_nand: case AtomicExpr::AO__c11_atomic_fetch_or: case AtomicExpr::AO__c11_atomic_fetch_xor: - case AtomicExpr::AO__c11_atomic_fetch_nand: - case AtomicExpr::AO__opencl_atomic_fetch_and: - case AtomicExpr::AO__opencl_atomic_fetch_or: - case AtomicExpr::AO__opencl_atomic_fetch_xor: - case AtomicExpr::AO__atomic_fetch_and: + case AtomicExpr::AO__c11_atomic_store: + case AtomicExpr::AO__c11_atomic_exchange: case AtomicExpr::AO__hip_atomic_fetch_and: - case AtomicExpr::AO__atomic_fetch_or: case AtomicExpr::AO__hip_atomic_fetch_or: - case AtomicExpr::AO__atomic_fetch_xor: case AtomicExpr::AO__hip_atomic_fetch_xor: - case AtomicExpr::AO__atomic_fetch_nand: - case AtomicExpr::AO__atomic_and_fetch: - case AtomicExpr::AO__atomic_or_fetch: - case AtomicExpr::AO__atomic_xor_fetch: - case AtomicExpr::AO__atomic_nand_fetch: + case AtomicExpr::AO__hip_atomic_store: + case AtomicExpr::AO__hip_atomic_exchange: + case AtomicExpr::AO__opencl_atomic_fetch_and: + case AtomicExpr::AO__opencl_atomic_fetch_or: + case AtomicExpr::AO__opencl_atomic_fetch_xor: + case AtomicExpr::AO__opencl_atomic_store: + case AtomicExpr::AO__opencl_atomic_exchange: Val1 = EmitValToTemp(*this, E->getVal1()); break; } @@ -1002,44 +1002,44 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { case AtomicExpr::AO__opencl_atomic_init: llvm_unreachable("Already handled above with EmitAtomicInit!"); -case AtomicExpr::AO__c11_atomic_fetch_add: -case AtomicExpr::AO__opencl_atomic_fetch_add: case AtomicExpr::AO__atomic_fetch_add: -case AtomicExpr::AO__hip_atomic_fetch_add: -case AtomicExpr::AO__c11_atomic_fetch_and: -case AtomicExpr::AO__opencl_atomic_fetch_and: -case AtomicExpr::AO__hip_atomic_fetch_and: case AtomicExpr::AO__atomic_fetch_and: -case AtomicExpr::AO__c11_atomic_fetch_or: -case AtomicExpr::AO__opencl_atomic_fetch_or: -case AtomicExpr::AO__h
[clang] [libcxxabi] [lld] [libcxx] [libunwind] [llvm] [compiler-rt] [clang-tools-extra] [msan][x86] Fix shadow if vararg overflow beyond kParamTLSSize (PR #72707)
https://github.com/vitalybuka edited https://github.com/llvm/llvm-project/pull/72707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [libcxxabi] [lld] [libcxx] [libunwind] [llvm] [compiler-rt] [clang-tools-extra] [msan][x86] Fix shadow if vararg overflow beyond kParamTLSSize (PR #72707)
https://github.com/vitalybuka closed https://github.com/llvm/llvm-project/pull/72707 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][NFC] Reorder Atomic builtins to be consistent. (PR #72718)
llvmbot wrote: @llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: None (Logikable) Changes --- Full diff: https://github.com/llvm/llvm-project/pull/72718.diff 1 Files Affected: - (modified) clang/lib/CodeGen/CGAtomic.cpp (+83-83) ``diff diff --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp index f7c597e181b0bd9..6005d5c51c0e1ac 100644 --- a/clang/lib/CodeGen/CGAtomic.cpp +++ b/clang/lib/CodeGen/CGAtomic.cpp @@ -861,10 +861,10 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { case AtomicExpr::AO__opencl_atomic_init: llvm_unreachable("Already handled above with EmitAtomicInit!"); + case AtomicExpr::AO__atomic_load_n: case AtomicExpr::AO__c11_atomic_load: case AtomicExpr::AO__opencl_atomic_load: case AtomicExpr::AO__hip_atomic_load: - case AtomicExpr::AO__atomic_load_n: break; case AtomicExpr::AO__atomic_load: @@ -880,14 +880,14 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { Dest = EmitPointerWithAlignment(E->getVal2()); break; - case AtomicExpr::AO__c11_atomic_compare_exchange_strong: + case AtomicExpr::AO__atomic_compare_exchange: + case AtomicExpr::AO__atomic_compare_exchange_n: case AtomicExpr::AO__c11_atomic_compare_exchange_weak: - case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: + case AtomicExpr::AO__c11_atomic_compare_exchange_strong: + case AtomicExpr::AO__hip_atomic_compare_exchange_weak: case AtomicExpr::AO__hip_atomic_compare_exchange_strong: case AtomicExpr::AO__opencl_atomic_compare_exchange_weak: - case AtomicExpr::AO__hip_atomic_compare_exchange_weak: - case AtomicExpr::AO__atomic_compare_exchange_n: - case AtomicExpr::AO__atomic_compare_exchange: + case AtomicExpr::AO__opencl_atomic_compare_exchange_strong: Val1 = EmitPointerWithAlignment(E->getVal1()); if (E->getOp() == AtomicExpr::AO__atomic_compare_exchange) Val2 = EmitPointerWithAlignment(E->getVal2()); @@ -938,32 +938,32 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { ShouldCastToIntPtrTy = !MemTy->isFloatingType(); [[fallthrough]]; - case AtomicExpr::AO__c11_atomic_store: - case AtomicExpr::AO__c11_atomic_exchange: - case AtomicExpr::AO__opencl_atomic_store: - case AtomicExpr::AO__hip_atomic_store: - case AtomicExpr::AO__opencl_atomic_exchange: - case AtomicExpr::AO__hip_atomic_exchange: + case AtomicExpr::AO__atomic_fetch_and: + case AtomicExpr::AO__atomic_fetch_nand: + case AtomicExpr::AO__atomic_fetch_or: + case AtomicExpr::AO__atomic_fetch_xor: + case AtomicExpr::AO__atomic_and_fetch: + case AtomicExpr::AO__atomic_nand_fetch: + case AtomicExpr::AO__atomic_or_fetch: + case AtomicExpr::AO__atomic_xor_fetch: case AtomicExpr::AO__atomic_store_n: case AtomicExpr::AO__atomic_exchange_n: case AtomicExpr::AO__c11_atomic_fetch_and: + case AtomicExpr::AO__c11_atomic_fetch_nand: case AtomicExpr::AO__c11_atomic_fetch_or: case AtomicExpr::AO__c11_atomic_fetch_xor: - case AtomicExpr::AO__c11_atomic_fetch_nand: - case AtomicExpr::AO__opencl_atomic_fetch_and: - case AtomicExpr::AO__opencl_atomic_fetch_or: - case AtomicExpr::AO__opencl_atomic_fetch_xor: - case AtomicExpr::AO__atomic_fetch_and: + case AtomicExpr::AO__c11_atomic_store: + case AtomicExpr::AO__c11_atomic_exchange: case AtomicExpr::AO__hip_atomic_fetch_and: - case AtomicExpr::AO__atomic_fetch_or: case AtomicExpr::AO__hip_atomic_fetch_or: - case AtomicExpr::AO__atomic_fetch_xor: case AtomicExpr::AO__hip_atomic_fetch_xor: - case AtomicExpr::AO__atomic_fetch_nand: - case AtomicExpr::AO__atomic_and_fetch: - case AtomicExpr::AO__atomic_or_fetch: - case AtomicExpr::AO__atomic_xor_fetch: - case AtomicExpr::AO__atomic_nand_fetch: + case AtomicExpr::AO__hip_atomic_store: + case AtomicExpr::AO__hip_atomic_exchange: + case AtomicExpr::AO__opencl_atomic_fetch_and: + case AtomicExpr::AO__opencl_atomic_fetch_or: + case AtomicExpr::AO__opencl_atomic_fetch_xor: + case AtomicExpr::AO__opencl_atomic_store: + case AtomicExpr::AO__opencl_atomic_exchange: Val1 = EmitValToTemp(*this, E->getVal1()); break; } @@ -1002,44 +1002,44 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) { case AtomicExpr::AO__opencl_atomic_init: llvm_unreachable("Already handled above with EmitAtomicInit!"); -case AtomicExpr::AO__c11_atomic_fetch_add: -case AtomicExpr::AO__opencl_atomic_fetch_add: case AtomicExpr::AO__atomic_fetch_add: -case AtomicExpr::AO__hip_atomic_fetch_add: -case AtomicExpr::AO__c11_atomic_fetch_and: -case AtomicExpr::AO__opencl_atomic_fetch_and: -case AtomicExpr::AO__hip_atomic_fetch_and: case AtomicExpr::AO__atomic_fetch_and: -case AtomicExpr::AO__c11_atomic_fetch_or: -case AtomicExpr::AO__opencl_atomic_fetch_or: -case AtomicExpr::AO__hip_atomic_fetch_or: -case AtomicExpr::AO__atomic_fetch_or: -case AtomicExpr::AO__c11_atomic_fetch_nand: +case AtomicExpr::AO__atomi
[clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
ilovepi wrote: Hi, we're seeing some breakages, similar to the above in our debugger tests with this patch. A failing bot can be found here: https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-debug/b8764552260903625809/overview You can find a fuller discussion in our bugtracker: https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=136182. The problem in our test is that the `DW_AT_const_value`, no longer seems to be in the expected place, similar to @dyung's issue above. Is there an ETA on when your fix will land? If it won't be soon, would you mind reverting until you can address this issue? https://github.com/llvm/llvm-project/pull/71780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
Michael137 wrote: > Hi, we're seeing some breakages, similar to the above in our debugger tests > with this patch. > > A failing bot can be found here: > https://luci-milo.appspot.com/ui/p/fuchsia/builders/ci/clang_toolchain.ci.core.x64-debug/b8764552260903625809/overview > > You can find a fuller discussion in our bugtracker: > https://bugs.fuchsia.dev/p/fuchsia/issues/detail?id=136182. > > The problem in our test is that the `DW_AT_const_value`, no longer seems to > be in the expected place, similar to @dyung's issue above. Is there an ETA on > when your fix will land? If it won't be soon, would you mind reverting until > you can address this issue? Sorry for not updating the thread. I'm about to open a PR for said change https://github.com/llvm/llvm-project/pull/71780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
ilovepi wrote: Fantastic! https://github.com/llvm/llvm-project/pull/71780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Option to ignore PP directives (PR #70338)
https://github.com/mydeveloperday edited https://github.com/llvm/llvm-project/pull/70338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Option to ignore PP directives (PR #70338)
https://github.com/mydeveloperday commented: Something here doesn't feel right https://github.com/llvm/llvm-project/pull/70338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format] Option to ignore PP directives (PR #70338)
@@ -24153,6 +24153,113 @@ TEST_F(FormatTest, WhitespaceSensitiveMacros) { verifyNoChange("FOO(String-ized&Messy+But: :Still=Intentional);", Style); } +TEST_F(FormatTest, IgnorePPDefinitions) { + FormatStyle Style = getLLVMStyle(); + Style.IgnorePPDefinitions = true; + + verifyNoChange("#define A", Style); + verifyNoChange("#define A b", Style); + verifyNoChange("#define A ( args )", Style); + verifyNoChange("#define A ( args ) = func ( args )", Style); + + verifyNoChange("#define A x:", Style); + verifyNoChange("#define A a. b", Style); + + // Surrounded with formatted code. + verifyFormat("int a;\n" + "#define A a\n" + "int a;", + "int a ;\n" mydeveloperday wrote: Why is that not formatted https://github.com/llvm/llvm-project/pull/70338 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format][NFC] Skip alignArrayInitializers() for 1-row matrices (PR #72166)
https://github.com/mydeveloperday approved this pull request. https://github.com/llvm/llvm-project/pull/72166 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-format]: Split alignment of declarations around assignment (PR #69340)
https://github.com/mydeveloperday approved this pull request. https://github.com/llvm/llvm-project/pull/69340 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [clangd] Don't show inlay hints for __builtin_dump_struct (PR #71366)
HighCommander4 wrote: > I see; it's a bit unfortunate that such an "invented" call expression gets an > ordinary `SourceLocation`. Out of curiosity, I dialled into today's LLVM [office hours](https://llvm.org/docs/GettingInvolved.html#office-hours) and asked Aaron Ballman about this. He also did not see an obvious way to tell apart the invented `CallExpr`s from regular `CallExpr`s, but suggested that it could be reasonable to just change the code for building the invented `CallExpr`s to give them an invalid SourceLocation (and then our inlay hints code should filter them out without further changes). https://github.com/llvm/llvm-project/pull/71366 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
Michael137 wrote: > Fantastic! @ilovepi To clarify the upcoming change, we are planning to attach the `DW_AT_const_value` to the definition, not the declaration like we used to. In case that's what your unit-test is expecting https://github.com/llvm/llvm-project/pull/71780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149573: [Clang][C++23] Implement core language changes from P1467R9 extended floating-point types and standard names
tahonermann added a comment. Apologies once again for the delayed response. I reviewed some today and will resume reviewing on Monday. In addition to the inline suggestions: `clang/docs/ReleaseNotes.rst` will need to be updated to reflect that the core language changes for P1467R9 have been implemented for `std::float16_t` and `std::bfloat16_t`. Given the confusing array of 16-bit floating-point types, how about modifying `clang/include/clang/AST/BuiltinTypes.def` to be more explicit regarding which is which? // 'half' in OpenCL, '__fp16' in ARM NEON. FLOATING_TYPE(Half, HalfTy) ... // '_Float16', 'std::float16_t' FLOATING_TYPE(Float16, HalfTy) // '__bf16', 'std::bfloat16_t' FLOATING_TYPE(BFloat16, BFloat16Ty) Hmm, having pasted the above, I now noticed that the `Half` and `Float16` types share the `HalfTy` singleton. Unless I'm mistaken, the changes being made will cause divergent behavior. Do these now need to be split? Comment at: clang/include/clang/AST/ASTContext.h:56 +// Conversion ranks introduced in C++23 6.8.6p2 [conv.rank] +enum FloatingRankCompareResult { + FRCR_Unordered, Naming suggestion: `FloatConvRankCompareResult`. Comment at: clang/include/clang/AST/ASTContext.h:1119 + CanQualType BFloat16Ty; // [C++23 6.8.3p5][basic.extended.fp] + CanQualType Float16Ty; // C11 extension ISO/IEC TS 18661-3 and [C++23 6.8.3p5][basic.extended.fp] CanQualType VoidPtrTy, NullPtrTy; I think the comment should reference http://eel.is/c++draft/basic.extended.fp#1 for `std::float16_t`. p5 is for `std::bfloat16_t`. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8931 >; +def err_cxx23_invalid_implicit_floating_point_cast : Error<"floating point cast results in loss of precision">; def err_typecheck_expect_scalar_operand : Error< The diagnostic text here looks more like the text of a warning. Since this is an error, I think it makes more sense to model the text on other similar errors and use "narrowing" or "implicit cast" terminology. For examples, see `err_spaceship_argument_narrowing` and `err_impcast_complex_scalar` Additionally, it would be helpful to include the relevant types in the message. Finally, line length should be kept to 80 characters or less per https://llvm.org/docs/CodingStandards.html#source-code-width. Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:779-788 +FloatingRankCompareResult order = Ctx.getFloatingTypeOrder(LTy, RTy); +if ((order == FRCR_Greater) || (order == FRCR_Equal_Greater_Subrank)) { RHS = (*doCast)(Solver, RHS, LTy, LBitWidth, RTy, RBitWidth); RTy = LTy; -} else if (order == 0) { +} else if ((order == FRCR_Equal) || (order == FRCR_Equal_Lesser_Subrank)) { LHS = (*doCast)(Solver, LHS, RTy, RBitWidth, LTy, LBitWidth); LTy = RTy; This looks like a pre-existing bug since, for standard floating-point types, narrowing implicit conversions are allowed. I think the intent was to add a cast on which ever side is needed, but the existing code instead adds a cast on the RHS when the LHS has a higher rank, adds a cast on the LHS when the LHS and RHS have the same rank, and wanders into UB when the RHS has a higher rank. The incorrect comparison goes back to when the code was introduced in commit 08f943c5630d8ee31d6a93227171d2f05db59e62. The introduction of unordered conversion ranks suggests additional changes are needed here, but it isn't clear to me what the right changes are. I added a suggested edit that adds an arbitrary cast (which probably suffices for the purposes of static analysis). Alternatively, an assert could be added for the unordered and equal cases. Comment at: clang/lib/Sema/SemaChecking.cpp:10451 return ICE->getCastKind() == CK_FloatingCast && - S.Context.getFloatingTypeOrder(From, To) < 0; + S.Context.getFloatingTypeOrder(From, To) == FRCR_Lesser; } codemzs wrote: > tahonermann wrote: > > I'm not sure this is right. If I understand correctly, the C++23 extended > > FP types don't participate in argument promotions. Perhaps they should be > > excluded here. > Rules for > [promotion](https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p1467r9.html#promotion) > are unchanged as per the proposal. This is just using the new enumeration to > represent a smaller conversion rank. I think this still produces an incorrect result in some cases though. According to http://eel.is/c++draft/conv.fpprom, the only floating-point promotion is from `float` to `double`. Ah, I think the prior code was incorrect, but non-symptomatic because it is only currently used in one place (in `CheckPrintfHandler::checkFormatExpr()`) and that code only cares about the integer cases. I would prefer that we fix this rather than extend the issue into
[clang] [lldb] Reland "[clang][DebugInfo] Emit global variable definitions for static data members with constant initializers" (PR #71780)
ilovepi wrote: I'm unclear on the specifics of the check, but it's probably something we can adjust if that is the long-term solution. CC @petrhosek Since he was interested in getting this resolved soon. https://github.com/llvm/llvm-project/pull/71780 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lldb] [llvm] [BOLT][DWARF] Fix handling of DWARF5 DWP (PR #72729)
https://github.com/ayermolo created https://github.com/llvm/llvm-project/pull/72729 Fixed handling of DWP as input. Before BOLT crashed. Now it will write out correct CU, and all the TUs. Potential future improvement is to scan all the TUs used in this CU, and only include those. >From 80adaca72cf869f8735d7a3684a8d64bc438e5b6 Mon Sep 17 00:00:00 2001 From: Amir Ayupov Date: Tue, 1 Jun 2021 11:37:41 -0700 Subject: [PATCH 1/3] Rebase: [Facebook] Add clang driver options to test debug info and BOLT Summary: This is an essential piece of infrastructure for us to be continuously testing debug info with BOLT. We can't only make changes to a test repo because we need to change debuginfo tests to call BOLT, hence, this diff needs to sit in our opensource repo. But when upstreaming to LLVM, this should be kept BOLT-only outside of LLVM. When upstreaming, we need to git diff and check all folders that are being modified by our commits and discard this one (and leave as an internal diff). To test BOLT in debuginfo tests, configure it with -DLLVM_TEST_BOLT=ON. Then run check-lldb and check-debuginfo. Manual rebase conflict history: https://phabricator.intern.facebook.com/D29205224 https://phabricator.intern.facebook.com/D29564078 https://phabricator.intern.facebook.com/D33289118 https://phabricator.intern.facebook.com/D34957174 https://phabricator.intern.facebook.com/D35317341 Test Plan: tested locally Configured with: -DLLVM_ENABLE_PROJECTS="clang;lld;lldb;compiler-rt;bolt;debuginfo-tests" -DLLVM_TEST_BOLT=ON Ran test suite with: ninja check-debuginfo ninja check-lldb Reviewers: maks, #llvm-bolt Reviewed By: maks Subscribers: ayermolo, phabricatorlinter Differential Revision: https://phabricator.intern.facebook.com/D46256657 Tasks: T92898286 --- clang/include/clang/Driver/Options.td | 4 clang/lib/Driver/ToolChains/Gnu.cpp| 29 ++ cross-project-tests/lit.cfg.py | 14 - cross-project-tests/lit.site.cfg.py.in | 4 lldb/test/API/lit.cfg.py | 5 + lldb/test/API/lit.site.cfg.py.in | 8 +++ lldb/test/Shell/helper/toolchain.py| 5 + lldb/test/Shell/lit.site.cfg.py.in | 9 llvm/CMakeLists.txt| 4 9 files changed, 81 insertions(+), 1 deletion(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 811550416110b3d..31ad86bc098ec63 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -5178,6 +5178,10 @@ def pg : Flag<["-"], "pg">, HelpText<"Enable mcount instrumentation">, MarshallingInfoFlag>; def pipe : Flag<["-", "--"], "pipe">, HelpText<"Use pipes between commands, when possible">; +// Facebook T92898286 +def post_link_optimize : Flag<["--"], "post-link-optimize">, + HelpText<"Apply post-link optimizations using BOLT">; +// End Facebook T92898286 def prebind__all__twolevel__modules : Flag<["-"], "prebind_all_twolevel_modules">; def prebind : Flag<["-"], "prebind">; def preload : Flag<["-"], "preload">; diff --git a/clang/lib/Driver/ToolChains/Gnu.cpp b/clang/lib/Driver/ToolChains/Gnu.cpp index ba95ce9c5a28153..bf55d90f6dc704b 100644 --- a/clang/lib/Driver/ToolChains/Gnu.cpp +++ b/clang/lib/Driver/ToolChains/Gnu.cpp @@ -666,12 +666,41 @@ void tools::gnutools::Linker::ConstructJob(Compilation &C, const JobAction &JA, } } + // Facebook T92898286 + if (Args.hasArg(options::OPT_post_link_optimize)) +CmdArgs.push_back("-q"); + // End Facebook T92898286 + Args.AddAllArgs(CmdArgs, options::OPT_T); const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath()); C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::AtFileCurCP(), Exec, CmdArgs, Inputs, Output)); + // Facebook T92898286 + if (!Args.hasArg(options::OPT_post_link_optimize) || !Output.isFilename()) +return; + + const char *MvExec = Args.MakeArgString(ToolChain.GetProgramPath("mv")); + ArgStringList MoveCmdArgs; + MoveCmdArgs.push_back(Output.getFilename()); + const char *PreBoltBin = + Args.MakeArgString(Twine(Output.getFilename()) + ".pre-bolt"); + MoveCmdArgs.push_back(PreBoltBin); + C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), + MvExec, MoveCmdArgs, std::nullopt)); + + ArgStringList BoltCmdArgs; + const char *BoltExec = + Args.MakeArgString(ToolChain.GetProgramPath("llvm-bolt")); + BoltCmdArgs.push_back(PreBoltBin); + BoltCmdArgs.push_back("-reorder-blocks=reverse"); + BoltCmdArgs.push_back("-update-debug-sections"); + BoltCmdArgs.push_back("-o"); + BoltCmdArgs.push_back(Output.getFilename()); + C.addCommand(std::make_unique(JA, *this, ResponseFileSupport::None(), + BoltExec, BoltCmdArgs, std::nullopt)); + // End Facebook T92898286 } void tools::gnutoo
[llvm] [clang] [lldb] [BOLT][DWARF] Fix handling of DWARF5 DWP (PR #72729)
llvmbot wrote: @llvm/pr-subscribers-clang-driver @llvm/pr-subscribers-clang Author: Alexander Yermolovich (ayermolo) Changes Fixed handling of DWP as input. Before BOLT crashed. Now it will write out correct CU, and all the TUs. Potential future improvement is to scan all the TUs used in this CU, and only include those. --- Patch is 80.02 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/72729.diff 25 Files Affected: - (modified) bolt/include/bolt/Core/DIEBuilder.h (+2-1) - (modified) bolt/lib/Core/BinaryEmitter.cpp (+1) - (modified) bolt/lib/Core/DIEBuilder.cpp (+9-8) - (modified) bolt/lib/Rewrite/DWARFRewriter.cpp (+6-8) - (added) bolt/test/X86/Inputs/dwarf5-df-types-dup-helper.s (+504) - (added) bolt/test/X86/Inputs/dwarf5-df-types-dup-main.s (+498) - (added) bolt/test/X86/dwarf5-df-types-dup-dwp-input.test (+29) - (modified) clang/include/clang/Driver/Options.td (+4) - (modified) clang/lib/Driver/ToolChains/Gnu.cpp (+29) - (modified) cross-project-tests/lit.cfg.py (+13-1) - (modified) cross-project-tests/lit.site.cfg.py.in (+4) - (modified) lldb/test/API/lit.cfg.py (+5) - (modified) lldb/test/API/lit.site.cfg.py.in (+8) - (modified) lldb/test/Shell/helper/toolchain.py (+5) - (modified) lldb/test/Shell/lit.site.cfg.py.in (+9) - (modified) llvm/CMakeLists.txt (+4) - (modified) llvm/include/llvm/MC/MCFragment.h (+22) - (modified) llvm/include/llvm/MC/MCObjectStreamer.h (+2) - (modified) llvm/include/llvm/MC/MCStreamer.h (+6) - (modified) llvm/lib/MC/MCAssembler.cpp (+81-37) - (modified) llvm/lib/MC/MCFragment.cpp (+12) - (modified) llvm/lib/MC/MCObjectStreamer.cpp (+5) - (modified) llvm/lib/MC/MCStreamer.cpp (+2) - (modified) llvm/lib/Target/X86/AsmParser/X86AsmParser.cpp (+24) - (added) llvm/test/MC/X86/directive-avoid_end_align.s (+208) ``diff diff --git a/bolt/include/bolt/Core/DIEBuilder.h b/bolt/include/bolt/Core/DIEBuilder.h index fb86e59468b835a..1c5252142d4ebf5 100644 --- a/bolt/include/bolt/Core/DIEBuilder.h +++ b/bolt/include/bolt/Core/DIEBuilder.h @@ -273,7 +273,8 @@ class DIEBuilder { void buildCompileUnits(const std::vector &CUs); /// Preventing implicit conversions. template void buildCompileUnits(T) = delete; - void buildBoth(); + /// Builds DWO Unit. For DWARF5 this includes the type units. + void buildDWOUnit(DWARFUnit &U); /// Returns DWARFUnitInfo for DWARFUnit DWARFUnitInfo &getUnitInfoByDwarfUnit(const DWARFUnit &DwarfUnit) { diff --git a/bolt/lib/Core/BinaryEmitter.cpp b/bolt/lib/Core/BinaryEmitter.cpp index fb1bf530c1974aa..82fbd8c0f67b215 100644 --- a/bolt/lib/Core/BinaryEmitter.cpp +++ b/bolt/lib/Core/BinaryEmitter.cpp @@ -482,6 +482,7 @@ void BinaryEmitter::emitFunctionBody(BinaryFunction &BF, FunctionFragment &FF, // This assumes the second instruction in the macro-op pair will get // assigned to its own MCRelaxableFragment. Since all JCC instructions // are relaxable, we should be safe. +Streamer.emitNeverAlignCodeAtEnd(/*Alignment to avoid=*/64, *BC.STI); } if (!EmitCodeOnly) { diff --git a/bolt/lib/Core/DIEBuilder.cpp b/bolt/lib/Core/DIEBuilder.cpp index 223ae714440d97d..6b33303ba553b72 100644 --- a/bolt/lib/Core/DIEBuilder.cpp +++ b/bolt/lib/Core/DIEBuilder.cpp @@ -193,12 +193,6 @@ void DIEBuilder::buildTypeUnits(const bool Init) { if (Init) BuilderState.reset(new State()); - unsigned int CUNum = getCUNum(DwarfContext, IsDWO); - getState().CloneUnitCtxMap.resize(CUNum); - DWARFContext::unit_iterator_range CU4TURanges = - IsDWO ? DwarfContext->dwo_types_section_units() -: DwarfContext->types_section_units(); - const DWARFUnitIndex &TUIndex = DwarfContext->getTUIndex(); if (!TUIndex.getRows().empty()) { for (auto &Row : TUIndex.getRows()) { @@ -208,6 +202,11 @@ void DIEBuilder::buildTypeUnits(const bool Init) { true); } } + unsigned int CUNum = getCUNum(DwarfContext, IsDWO); + getState().CloneUnitCtxMap.resize(CUNum); + DWARFContext::unit_iterator_range CU4TURanges = + IsDWO ? DwarfContext->dwo_types_section_units() +: DwarfContext->types_section_units(); getState().Type = ProcessingType::DWARF4TUs; for (std::unique_ptr &DU : CU4TURanges) @@ -278,11 +277,13 @@ void DIEBuilder::buildCompileUnits(const std::vector &CUs) { constructFromUnit(*DU); } -void DIEBuilder::buildBoth() { +void DIEBuilder::buildDWOUnit(DWARFUnit &U) { BuilderState.release(); BuilderState = std::make_unique(); buildTypeUnits(false); - buildCompileUnits(false); + getState().Type = ProcessingType::CUs; + registerUnit(U, false); + constructFromUnit(U); } DIE *DIEBuilder::constructDIEFast(DWARFDie &DDie, DWARFUnit &U, diff --git a/bolt/lib/Rewrite/DWARFRewriter.cpp b/bolt/lib/Rewrite/DWARFRewriter.cpp index 360b82f45bd7754..3bba61724b9e542 100644 --- a/bolt/lib/Rewrite/DWARFRewriter.cpp +++ b/bolt/lib/Rewri
[clang] [clang][DebugInfo] Attach DW_AT_const_value to static data-member definitions if available (PR #72730)
https://github.com/Michael137 created https://github.com/llvm/llvm-project/pull/72730 In https://github.com/llvm/llvm-project/pull/71780 we started emitting definitions for all static data-members with constant initialisers, even if they were constants (i.e., didn't have a location). We also dropped the `DW_AT_const_value` from the declaration to help resolve [inconsistencies during type merging in the DWARFParallelLinker](https://github.com/llvm/llvm-project/pull/68721). However, for static data members that do have locations, we wouldn't emit a `DW_AT_const_value` on it, assuming that the consumer knows how to read the value using the location. This broke some consumers that really wanted to find a `DW_AT_const_value`. This patch makes sure we also attach the `DW_AT_const_value` to definitions that have a location. >From 6dcb09dcc50a9b9e92640412242927b3e226929e Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Sat, 18 Nov 2023 00:20:05 + Subject: [PATCH 1/3] [clang][DebugInfo][NFC] Create evaluateConstantInitializer helper function --- clang/lib/CodeGen/CGDebugInfo.cpp | 18 ++ 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0b52d99ad07f164..4840581b5d03f89 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -69,6 +69,19 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) { return D->hasAttr() ? D->getMaxAlignment() : 0; } +APValue const * evaluateConstantInitializer(clang::VarDecl const * VD) { + assert (VD != nullptr); + + VD = VD->getCanonicalDecl(); + if (!VD) +return nullptr; + + if (!VD->hasConstantInitialization() || !VD->hasInit()) +return nullptr; + + return VD->evaluateValue(); +} + CGDebugInfo::CGDebugInfo(CodeGenModule &CGM) : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()), DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs), @@ -5596,14 +5609,11 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) { if (VD->hasAttr()) return; - if (!VD->hasInit()) -return; - const auto CacheIt = DeclCache.find(VD); if (CacheIt != DeclCache.end()) return; - auto const *InitVal = VD->evaluateValue(); + auto const * InitVal = evaluateConstantInitializer(VD); if (!InitVal) return; >From fcc6e19d108798fb18c1973e4d4cc3800da07f9f Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Sat, 18 Nov 2023 00:52:24 + Subject: [PATCH 2/3] fixup! clang-format --- clang/lib/CodeGen/CGDebugInfo.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 4840581b5d03f89..9bba6e6b13e9318 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -69,8 +69,8 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) { return D->hasAttr() ? D->getMaxAlignment() : 0; } -APValue const * evaluateConstantInitializer(clang::VarDecl const * VD) { - assert (VD != nullptr); +APValue const *evaluateConstantInitializer(clang::VarDecl const *VD) { + assert(VD != nullptr); VD = VD->getCanonicalDecl(); if (!VD) @@ -5613,7 +5613,7 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) { if (CacheIt != DeclCache.end()) return; - auto const * InitVal = evaluateConstantInitializer(VD); + auto const *InitVal = evaluateConstantInitializer(VD); if (!InitVal) return; >From 148ab1793a866111060f77807ff065040fad92d8 Mon Sep 17 00:00:00 2001 From: Michael Buch Date: Sat, 18 Nov 2023 00:22:06 + Subject: [PATCH 3/3] [clang][DebugInfo] Attach DW_AT_const_value to static data-member definitions if available --- clang/lib/CodeGen/CGDebugInfo.cpp | 10 -- .../CodeGenCXX/debug-info-static-inline-member.cpp | 2 +- clang/test/CodeGenCXX/inline-dllexport-member.cpp | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 9bba6e6b13e9318..e01c57baef19931 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -5516,11 +5516,17 @@ void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var, } AppendAddressSpaceXDeref(AddressSpace, Expr); +llvm::DIExpression *E = nullptr; +if (Expr.empty()) { + if (auto const *InitVal = evaluateConstantInitializer(D)) +E = createConstantValueExpression(D, *InitVal); +} else + E = DBuilder.createExpression(Expr); + llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D); GVE = DBuilder.createGlobalVariableExpression( DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit), -Var->hasLocalLinkage(), true, -Expr.empty() ? nullptr : DBuilder.createExpression(Expr), +Var->hasLoca
[clang] [clang][DebugInfo] Attach DW_AT_const_value to static data-member definitions if available (PR #72730)
llvmbot wrote: @llvm/pr-subscribers-clang-codegen Author: Michael Buch (Michael137) Changes In https://github.com/llvm/llvm-project/pull/71780 we started emitting definitions for all static data-members with constant initialisers, even if they were constants (i.e., didn't have a location). We also dropped the `DW_AT_const_value` from the declaration to help resolve [inconsistencies during type merging in the DWARFParallelLinker](https://github.com/llvm/llvm-project/pull/68721). However, for static data members that do have locations, we wouldn't emit a `DW_AT_const_value` on it, assuming that the consumer knows how to read the value using the location. This broke some consumers that really wanted to find a `DW_AT_const_value`. This patch makes sure we also attach the `DW_AT_const_value` to definitions that have a location. --- Full diff: https://github.com/llvm/llvm-project/pull/72730.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+22-6) - (modified) clang/test/CodeGenCXX/debug-info-static-inline-member.cpp (+1-1) - (modified) clang/test/CodeGenCXX/inline-dllexport-member.cpp (+1-1) ``diff diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0b52d99ad07f164..e01c57baef19931 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -69,6 +69,19 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) { return D->hasAttr() ? D->getMaxAlignment() : 0; } +APValue const *evaluateConstantInitializer(clang::VarDecl const *VD) { + assert(VD != nullptr); + + VD = VD->getCanonicalDecl(); + if (!VD) +return nullptr; + + if (!VD->hasConstantInitialization() || !VD->hasInit()) +return nullptr; + + return VD->evaluateValue(); +} + CGDebugInfo::CGDebugInfo(CodeGenModule &CGM) : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()), DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs), @@ -5503,11 +5516,17 @@ void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var, } AppendAddressSpaceXDeref(AddressSpace, Expr); +llvm::DIExpression *E = nullptr; +if (Expr.empty()) { + if (auto const *InitVal = evaluateConstantInitializer(D)) +E = createConstantValueExpression(D, *InitVal); +} else + E = DBuilder.createExpression(Expr); + llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D); GVE = DBuilder.createGlobalVariableExpression( DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit), -Var->hasLocalLinkage(), true, -Expr.empty() ? nullptr : DBuilder.createExpression(Expr), +Var->hasLocalLinkage(), true, E, getOrCreateStaticDataMemberDeclarationOrNull(D), TemplateParameters, Align, Annotations); Var->addDebugInfo(GVE); @@ -5596,14 +5615,11 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) { if (VD->hasAttr()) return; - if (!VD->hasInit()) -return; - const auto CacheIt = DeclCache.find(VD); if (CacheIt != DeclCache.end()) return; - auto const *InitVal = VD->evaluateValue(); + auto const *InitVal = evaluateConstantInitializer(VD); if (!InitVal) return; diff --git a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp index f2d4d9408a8297a..950ea9b302b290c 100644 --- a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp +++ b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp @@ -43,7 +43,7 @@ int main() { // CHECK: @{{.*}}cexpr_struct_with_addr{{.*}} = // CHECK-SAME!dbg ![[EMPTY_GLOBAL:[0-9]+]] -// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr: !DIExpression()) +// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 25, DW_OP_stack_value)) // CHECK: ![[INT_VAR]] = distinct !DIGlobalVariable(name: "cexpr_int_with_addr", linkageName: // CHECK-SAME:isLocal: false, isDefinition: true, declaration: ![[INT_DECL:[0-9]+]]) diff --git a/clang/test/CodeGenCXX/inline-dllexport-member.cpp b/clang/test/CodeGenCXX/inline-dllexport-member.cpp index d6b004d66dc6cbd..6bc01599c466780 100644 --- a/clang/test/CodeGenCXX/inline-dllexport-member.cpp +++ b/clang/test/CodeGenCXX/inline-dllexport-member.cpp @@ -7,7 +7,7 @@ struct __declspec(dllexport) s { static const unsigned int ui = 0; }; -// CHECK: [[UI]] = !DIGlobalVariableExpression(var: [[UIV:.*]], expr: !DIExpression()) +// CHECK: [[UI]] = !DIGlobalVariableExpression(var: [[UIV:.*]], expr: !DIExpression(DW_OP_constu, 0, DW_OP_stack_value)) // CHECK: [[UIV]] = distinct !DIGlobalVariable(name: "ui", linkageName: "?ui@s@@2IB", scope: ![[SCOPE:[0-9]+]], // CHECK: ![[SCOPE]] = distinct !DICompileUnit( `` https://github.com/llvm/llvm-project/pull/72730 ___ cfe-commit
[clang] [clang][DebugInfo] Attach DW_AT_const_value to static data-member definitions if available (PR #72730)
llvmbot wrote: @llvm/pr-subscribers-debuginfo Author: Michael Buch (Michael137) Changes In https://github.com/llvm/llvm-project/pull/71780 we started emitting definitions for all static data-members with constant initialisers, even if they were constants (i.e., didn't have a location). We also dropped the `DW_AT_const_value` from the declaration to help resolve [inconsistencies during type merging in the DWARFParallelLinker](https://github.com/llvm/llvm-project/pull/68721). However, for static data members that do have locations, we wouldn't emit a `DW_AT_const_value` on it, assuming that the consumer knows how to read the value using the location. This broke some consumers that really wanted to find a `DW_AT_const_value`. This patch makes sure we also attach the `DW_AT_const_value` to definitions that have a location. --- Full diff: https://github.com/llvm/llvm-project/pull/72730.diff 3 Files Affected: - (modified) clang/lib/CodeGen/CGDebugInfo.cpp (+22-6) - (modified) clang/test/CodeGenCXX/debug-info-static-inline-member.cpp (+1-1) - (modified) clang/test/CodeGenCXX/inline-dllexport-member.cpp (+1-1) ``diff diff --git a/clang/lib/CodeGen/CGDebugInfo.cpp b/clang/lib/CodeGen/CGDebugInfo.cpp index 0b52d99ad07f164..e01c57baef19931 100644 --- a/clang/lib/CodeGen/CGDebugInfo.cpp +++ b/clang/lib/CodeGen/CGDebugInfo.cpp @@ -69,6 +69,19 @@ static uint32_t getDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) { return D->hasAttr() ? D->getMaxAlignment() : 0; } +APValue const *evaluateConstantInitializer(clang::VarDecl const *VD) { + assert(VD != nullptr); + + VD = VD->getCanonicalDecl(); + if (!VD) +return nullptr; + + if (!VD->hasConstantInitialization() || !VD->hasInit()) +return nullptr; + + return VD->evaluateValue(); +} + CGDebugInfo::CGDebugInfo(CodeGenModule &CGM) : CGM(CGM), DebugKind(CGM.getCodeGenOpts().getDebugInfo()), DebugTypeExtRefs(CGM.getCodeGenOpts().DebugTypeExtRefs), @@ -5503,11 +5516,17 @@ void CGDebugInfo::EmitGlobalVariable(llvm::GlobalVariable *Var, } AppendAddressSpaceXDeref(AddressSpace, Expr); +llvm::DIExpression *E = nullptr; +if (Expr.empty()) { + if (auto const *InitVal = evaluateConstantInitializer(D)) +E = createConstantValueExpression(D, *InitVal); +} else + E = DBuilder.createExpression(Expr); + llvm::DINodeArray Annotations = CollectBTFDeclTagAnnotations(D); GVE = DBuilder.createGlobalVariableExpression( DContext, DeclName, LinkageName, Unit, LineNo, getOrCreateType(T, Unit), -Var->hasLocalLinkage(), true, -Expr.empty() ? nullptr : DBuilder.createExpression(Expr), +Var->hasLocalLinkage(), true, E, getOrCreateStaticDataMemberDeclarationOrNull(D), TemplateParameters, Align, Annotations); Var->addDebugInfo(GVE); @@ -5596,14 +5615,11 @@ void CGDebugInfo::EmitGlobalVariable(const VarDecl *VD) { if (VD->hasAttr()) return; - if (!VD->hasInit()) -return; - const auto CacheIt = DeclCache.find(VD); if (CacheIt != DeclCache.end()) return; - auto const *InitVal = VD->evaluateValue(); + auto const *InitVal = evaluateConstantInitializer(VD); if (!InitVal) return; diff --git a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp index f2d4d9408a8297a..950ea9b302b290c 100644 --- a/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp +++ b/clang/test/CodeGenCXX/debug-info-static-inline-member.cpp @@ -43,7 +43,7 @@ int main() { // CHECK: @{{.*}}cexpr_struct_with_addr{{.*}} = // CHECK-SAME!dbg ![[EMPTY_GLOBAL:[0-9]+]] -// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr: !DIExpression()) +// CHECK: !DIGlobalVariableExpression(var: ![[INT_VAR:[0-9]+]], expr: !DIExpression(DW_OP_constu, 25, DW_OP_stack_value)) // CHECK: ![[INT_VAR]] = distinct !DIGlobalVariable(name: "cexpr_int_with_addr", linkageName: // CHECK-SAME:isLocal: false, isDefinition: true, declaration: ![[INT_DECL:[0-9]+]]) diff --git a/clang/test/CodeGenCXX/inline-dllexport-member.cpp b/clang/test/CodeGenCXX/inline-dllexport-member.cpp index d6b004d66dc6cbd..6bc01599c466780 100644 --- a/clang/test/CodeGenCXX/inline-dllexport-member.cpp +++ b/clang/test/CodeGenCXX/inline-dllexport-member.cpp @@ -7,7 +7,7 @@ struct __declspec(dllexport) s { static const unsigned int ui = 0; }; -// CHECK: [[UI]] = !DIGlobalVariableExpression(var: [[UIV:.*]], expr: !DIExpression()) +// CHECK: [[UI]] = !DIGlobalVariableExpression(var: [[UIV:.*]], expr: !DIExpression(DW_OP_constu, 0, DW_OP_stack_value)) // CHECK: [[UIV]] = distinct !DIGlobalVariable(name: "ui", linkageName: "?ui@s@@2IB", scope: ![[SCOPE:[0-9]+]], // CHECK: ![[SCOPE]] = distinct !DICompileUnit( `` https://github.com/llvm/llvm-project/pull/72730 ___ cfe-commits ma