[Lldb-commits] [PATCH] D139158: [LLDB][LoongArch] Make software single stepping work

[Tiezhu Yang via Phabricator via lldb-commits]

seehearfeel added inline comments.



Comment at: 
lldb/source/Plugins/Instruction/LoongArch/EmulateInstructionLoongArch.h:58
+   uint32_t reg_num) override;
+  lldb::addr_t ReadPC(bool *success);
+  bool WritePC(lldb::addr_t pc);

lh03061238 wrote:
> DavidSpickett wrote:
> > I think the older targets use this form but for riscv they went with 
> > `llvm::Optional ReadPC();` which I prefer over pointer plus 
> > addr_t.
> > I think the older targets use this form but for riscv they went with 
> > `llvm::Optional ReadPC();` which I prefer over pointer plus 
> > addr_t.
> 
> EmulateInstructionLoongArch is relatively simple Compared with riscv. If use 
> llvm::Optional ReadPC(), There will be more type 
> conversions here. I prefer to keep that definition for now, Considering the 
> complexity of the code at this stage.
> 
> Thank you.
@DavidSpickett Are you OK with the current code?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139158/new/

https://reviews.llvm.org/D139158

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] e0fdc56 - [lldb] Don't use Optional::getPointer (NFC)

[Kazu Hirata via lldb-commits]

Author: Kazu Hirata
Date: 2022-12-07T17:42:59-08:00
New Revision: e0fdc563585f38e7b20507b541dacbfe58aa904b

URL: 
https://github.com/llvm/llvm-project/commit/e0fdc563585f38e7b20507b541dacbfe58aa904b
DIFF: 
https://github.com/llvm/llvm-project/commit/e0fdc563585f38e7b20507b541dacbfe58aa904b.diff

LOG: [lldb] Don't use Optional::getPointer (NFC)

Note that Optional::getPointer has been deprecated since commit
80145dcb011b03a7c54fdfaa3a7beeaf5c18863a on November 23, 2022.

Added: 


Modified: 

lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp

Removed: 




diff  --git 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
index 6047ae5a115e8..0c2e5237f600c 100644
--- 
a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
+++ 
b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
@@ -194,7 +194,7 @@ Status 
GDBRemoteCommunicationServerPlatform::LaunchGDBServer(
 #if !defined(__APPLE__)
   url << m_socket_scheme << "://";
 #endif
-  uint16_t *port_ptr = port.getPointer();
+  uint16_t *port_ptr = &*port;
   if (m_socket_protocol == Socket::ProtocolTcp) {
 std::string platform_uri = GetConnection()->GetURI();
 llvm::Optional parsed_uri = URI::Parse(platform_uri);



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo abandoned this revision.
ayermolo added a comment.

Redid in D139577 .


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139577: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 481047.
ayermolo added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139577/new/

https://reviews.llvm.org/D139577

Files:
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DWP/DWP.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/DebugInfo/dwarfdump-dwp.test
  llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
  llvm/test/tools/llvm-dwp/X86/info-v5.s
  llvm/test/tools/llvm-dwp/X86/loclists.s
  llvm/test/tools/llvm-dwp/X86/merge.test
  llvm/test/tools/llvm-dwp/X86/rnglists.s
  llvm/test/tools/llvm-dwp/X86/simple.test
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Index: llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
===
--- llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
+++ llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
@@ -17,7 +17,7 @@
 # CHECK:  Index Signature INFO ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
+# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
 # CHECK-NOT:  [
 
 # CHECK:  .debug_tu_index contents:
@@ -25,7 +25,7 @@
 # CHECK:  Index Signature TYPES ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
+# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
 # CHECK-NOT:  [
 
 .section .debug_abbrev.dwo, "e", @progbits
Index: llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
===
--- llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
+++ llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
@@ -13,9 +13,9 @@
 # CHECK: 0x001b: Type Unit: length = 0x0017, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_type, abbr_offset = 0x, addr_size = 0x08, name = '', type_signature = [[TUID2:.*]], type_offset = 0x0019 (next unit at 0x0036)
 # CHECK-DAG: .debug_tu_index contents:
 # CHECK: version = 5, units = 2, slots = 4
-# CHECK: Index Signature  INFO ABBREV
-# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
-# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
+# CHECK: Index Signature  INFO ABBREV
+# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
+# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
 
 .section	.debug_info.dwo,"e",@progbits
 .long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
Index: llvm/test/tools/llvm-dwp/X86/simple.test
===
--- llvm/test/tools/llvm-dwp/X86/simple.test
+++ llvm/test/tools/llvm-dwp/X86/simple.test
@@ -28,9 +28,9 @@
 CHECK: DW_TAG_formal_parameter
 
 CHECK: .debug_info.dwo contents:
-CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK: 0x[[#%.8x,AOFF:]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at [[BOFF:.*]])
+CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,BOFF:]])
 CHECK: DW_TAG_compile_unit
 CHECK:   DW_AT_name {{.*}} "a.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOA:.*]])
@@ -40,9 +40,9 @@
 NOTYP: DW_AT_name {{.*}} "foo"
 TYPES: DW_AT_signature {{.*}} ([[FOOSIG:.*]])
 
-CHECK: [[BOFF]]:
+CHECK: 0x[[#BOFF]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at [[XOFF:.*]])
+CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,XOFF:]])
 CHECK:   DW_AT_name {{.*}} "b.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOB:.*]])
 CHECK:   DW_TAG_structure_type
@@ -54,32 +54,32 @@
 
 NOTYP-NOT: .debug_types.dwo contents:
 TYPES-LABEL: .debug_types.dwo contents:
-TYPES: [[FOOUOFF:0x[0-9a-f]*]]:
+TYPES: 0x[[#%.8x,FOOUOFF:]]:
 TYPES-LABEL: Type Unit: length = 0x0020, format = DWARF32, version = 0x0004, abbr_offset =
-TYPES: 0x[[AAOFF]], addr_size = 0x08, name = 'foo', type_signature = [[FOOSIG]], type_offset = 

[Lldb-commits] [PATCH] D139577: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

Re-did D139379  after pushing it by mistake.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139577/new/

https://reviews.llvm.org/D139577

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139577: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo created this revision.
Herald added subscribers: mstorsjo, hoy, modimo, wenlei, arphaman, hiraditya.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a project: All.
ayermolo requested review of this revision.
Herald added subscribers: llvm-commits, lldb-commits, yota9.
Herald added projects: LLDB, LLVM.

Changed contribution data structure to 64 bit. I added the 32bit and 64bit
accessors to make it explicit where we use 32bit and where we use 64bit. Also to
make sure sure we catch all the cases where this data structure is used.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139577

Files:
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DWP/DWP.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/DebugInfo/dwarfdump-dwp.test
  llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
  llvm/test/tools/llvm-dwp/X86/info-v5.s
  llvm/test/tools/llvm-dwp/X86/loclists.s
  llvm/test/tools/llvm-dwp/X86/merge.test
  llvm/test/tools/llvm-dwp/X86/rnglists.s
  llvm/test/tools/llvm-dwp/X86/simple.test
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Index: llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
===
--- llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
+++ llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
@@ -17,7 +17,7 @@
 # CHECK:  Index Signature INFO ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
+# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
 # CHECK-NOT:  [
 
 # CHECK:  .debug_tu_index contents:
@@ -25,7 +25,7 @@
 # CHECK:  Index Signature TYPES ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
+# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
 # CHECK-NOT:  [
 
 .section .debug_abbrev.dwo, "e", @progbits
Index: llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
===
--- llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
+++ llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
@@ -13,9 +13,9 @@
 # CHECK: 0x001b: Type Unit: length = 0x0017, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_type, abbr_offset = 0x, addr_size = 0x08, name = '', type_signature = [[TUID2:.*]], type_offset = 0x0019 (next unit at 0x0036)
 # CHECK-DAG: .debug_tu_index contents:
 # CHECK: version = 5, units = 2, slots = 4
-# CHECK: Index Signature  INFO ABBREV
-# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
-# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
+# CHECK: Index Signature  INFO ABBREV
+# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
+# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
 
 .section	.debug_info.dwo,"e",@progbits
 .long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
Index: llvm/test/tools/llvm-dwp/X86/simple.test
===
--- llvm/test/tools/llvm-dwp/X86/simple.test
+++ llvm/test/tools/llvm-dwp/X86/simple.test
@@ -28,9 +28,9 @@
 CHECK: DW_TAG_formal_parameter
 
 CHECK: .debug_info.dwo contents:
-CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK: 0x[[#%.8x,AOFF:]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at [[BOFF:.*]])
+CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,BOFF:]])
 CHECK: DW_TAG_compile_unit
 CHECK:   DW_AT_name {{.*}} "a.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOA:.*]])
@@ -40,9 +40,9 @@
 NOTYP: DW_AT_name {{.*}} "foo"
 TYPES: DW_AT_signature {{.*}} ([[FOOSIG:.*]])
 
-CHECK: [[BOFF]]:
+CHECK: 0x[[#BOFF]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at [[XOFF:.*]])
+CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,XOFF:]])
 CHECK:   DW_AT_name 

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo added a comment.

Accidentally pushed when pushing bolt changes. Reverted.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139379: [llvm][dwwarf] Change CU/TU index to 64-bit

[Alexander Yermolovich via Phabricator via lldb-commits]

ayermolo updated this revision to Diff 480994.
ayermolo added a comment.

rebase, clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

Files:
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h
  llvm/include/llvm/DebugInfo/DWARF/DWARFUnitIndex.h
  llvm/lib/DWP/DWP.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp
  llvm/lib/DebugInfo/DWARF/DWARFUnitIndex.cpp
  llvm/lib/DebugInfo/DWARF/DWARFVerifier.cpp
  llvm/test/DebugInfo/X86/debug-cu-index-unknown-section.s
  llvm/test/DebugInfo/X86/dwp-v2-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v2-tu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-cu-index.s
  llvm/test/DebugInfo/X86/dwp-v5-tu-index.s
  llvm/test/DebugInfo/dwarfdump-dwp.test
  llvm/test/tools/llvm-dwp/X86/debug_macro_v5.s
  llvm/test/tools/llvm-dwp/X86/info-v5.s
  llvm/test/tools/llvm-dwp/X86/loclists.s
  llvm/test/tools/llvm-dwp/X86/merge.test
  llvm/test/tools/llvm-dwp/X86/rnglists.s
  llvm/test/tools/llvm-dwp/X86/simple.test
  llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
  llvm/test/tools/llvm-dwp/X86/unknown-section-id.s

Index: llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
===
--- llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
+++ llvm/test/tools/llvm-dwp/X86/unknown-section-id.s
@@ -17,7 +17,7 @@
 # CHECK:  Index Signature INFO ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
+# CHECK-NEXT: 1 0x1122 [0x, 0x0014) [0x, 0x0009)
 # CHECK-NOT:  [
 
 # CHECK:  .debug_tu_index contents:
@@ -25,7 +25,7 @@
 # CHECK:  Index Signature TYPES ABBREV
 # CHECK-NOT:  Unknown
 # CHECK:  -
-# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
+# CHECK-NEXT: 2 0x1133 [0x, 0x0019) [0x0009, 0x0014)
 # CHECK-NOT:  [
 
 .section .debug_abbrev.dwo, "e", @progbits
Index: llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
===
--- llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
+++ llvm/test/tools/llvm-dwp/X86/tu_units_v5.s
@@ -13,9 +13,9 @@
 # CHECK: 0x001b: Type Unit: length = 0x0017, format = DWARF32, version = 0x0005, unit_type = DW_UT_split_type, abbr_offset = 0x, addr_size = 0x08, name = '', type_signature = [[TUID2:.*]], type_offset = 0x0019 (next unit at 0x0036)
 # CHECK-DAG: .debug_tu_index contents:
 # CHECK: version = 5, units = 2, slots = 4
-# CHECK: Index Signature  INFO ABBREV
-# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
-# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
+# CHECK: Index Signature  INFO ABBREV
+# CHECK: 1 [[TUID1]]  [0x, 0x001b) [0x, 0x0010)
+# CHECK: 4 [[TUID2]]  [0x001b, 0x0036) [0x, 0x0010)
 
 .section	.debug_info.dwo,"e",@progbits
 .long	.Ldebug_info_dwo_end0-.Ldebug_info_dwo_start0 # Length of Unit
Index: llvm/test/tools/llvm-dwp/X86/simple.test
===
--- llvm/test/tools/llvm-dwp/X86/simple.test
+++ llvm/test/tools/llvm-dwp/X86/simple.test
@@ -28,9 +28,9 @@
 CHECK: DW_TAG_formal_parameter
 
 CHECK: .debug_info.dwo contents:
-CHECK: [[AOFF:0x[0-9a-f]*]]:
+CHECK: 0x[[#%.8x,AOFF:]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at [[BOFF:.*]])
+CHECK: 0x[[AAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,BOFF:]])
 CHECK: DW_TAG_compile_unit
 CHECK:   DW_AT_name {{.*}} "a.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOA:.*]])
@@ -40,9 +40,9 @@
 NOTYP: DW_AT_name {{.*}} "foo"
 TYPES: DW_AT_signature {{.*}} ([[FOOSIG:.*]])
 
-CHECK: [[BOFF]]:
+CHECK: 0x[[#BOFF]]:
 CHECK-LABEL: Compile Unit: length = {{.*}}, version = 0x0004, abbr_offset =
-CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at [[XOFF:.*]])
+CHECK: 0x[[BAOFF]], addr_size = 0x08 (next unit at 0x[[#%.8x,XOFF:]])
 CHECK:   DW_AT_name {{.*}} "b.cpp"
 CHECK:   DW_AT_GNU_dwo_id {{.*}} ([[DWOB:.*]])
 CHECK:   DW_TAG_structure_type
@@ -54,32 +54,32 @@
 
 NOTYP-NOT: .debug_types.dwo contents:
 TYPES-LABEL: .debug_types.dwo contents:
-TYPES: [[FOOUOFF:0x[0-9a-f]*]]:
+TYPES: 0x[[#%.8x,FOOUOFF:]]:
 TYPES-LABEL: Type Unit: length = 0x0020, format = DWARF32, version = 0x0004, abbr_offset =
-TYPES: 0x[[AAOFF]], addr_size = 0x08, name = 'foo', type_signature = [[FOOSIG]], 

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.



Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:18
+  std::unique_lock lock(mutex);
+  cv.notify_one();
+  n = foo(n);

Why do you need this initial notification?



Comment at: lldb/test/API/functionalities/scripted_process/main.cpp:25-27
+  while (baz(n, mutex, cv, done) != 42) {
+;
+  }

Why does this need to be in a while loop? The condition_variable's `wait` 
function should guarantee that `baz` only ever returns `42`, no? Unless I'm 
missing something.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139484/new/

https://reviews.llvm.org/D139484

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139461: Fix dwarf5-lazy-dwo.c for the default c target not being c99.

[Mitch Phillips via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc5bda52f00b: Fix dwarf5-lazy-dwo.c for the default c target 
not being c99. (authored by hctim).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139461/new/

https://reviews.llvm.org/D139461

Files:
  lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c


Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
===
--- lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
@@ -3,11 +3,11 @@
 // -gsplit-dwarf is supported only on Linux.
 // REQUIRES: system-linux
 
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t1.o -DONE
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t2.o -DTWO
-// RUN: %clang_host %t1.o %t2.o -o %t
+// RUN: %clang_host %t1.o %t2.o -o %t -std=c99
 // RUN: %lldb %t -o "log enable 'lldb' object" -o "settings set 
stop-line-count-before 0" \
 // RUN:   -o "b main" -o "run" -o "image lookup -n main -v" -b | FileCheck %s
 


Index: lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
===
--- lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
+++ lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
@@ -3,11 +3,11 @@
 // -gsplit-dwarf is supported only on Linux.
 // REQUIRES: system-linux
 
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t1.o -DONE
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t2.o -DTWO
-// RUN: %clang_host %t1.o %t2.o -o %t
+// RUN: %clang_host %t1.o %t2.o -o %t -std=c99
 // RUN: %lldb %t -o "log enable 'lldb' object" -o "settings set stop-line-count-before 0" \
 // RUN:   -o "b main" -o "run" -o "image lookup -n main -v" -b | FileCheck %s
 
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] fc5bda5 - Fix dwarf5-lazy-dwo.c for the default c target not being c99.

[Mitch Phillips via lldb-commits]

Author: Mitch Phillips
Date: 2022-12-07T09:07:56-08:00
New Revision: fc5bda52f00b63fbe4b503b813380d9b81ee7f0b

URL: 
https://github.com/llvm/llvm-project/commit/fc5bda52f00b63fbe4b503b813380d9b81ee7f0b
DIFF: 
https://github.com/llvm/llvm-project/commit/fc5bda52f00b63fbe4b503b813380d9b81ee7f0b.diff

LOG: Fix dwarf5-lazy-dwo.c for the default c target not being c99.

My host compiler is clang version 15.0.0, which uses -std=c11 by
default. The test asserts that the language is 'c99', and so the test
fails locally.

Update the test to be explicit about compiling with 'c99'.

Reviewed By: Eric

Differential Revision: https://reviews.llvm.org/D139461

Added: 


Modified: 
lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c

Removed: 




diff  --git a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c 
b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
index 5c9f1fc60976e..d9c823793cc12 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
+++ b/lldb/test/Shell/SymbolFile/DWARF/dwarf5-lazy-dwo.c
@@ -3,11 +3,11 @@
 // -gsplit-dwarf is supported only on Linux.
 // REQUIRES: system-linux
 
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t1.o -DONE
-// RUN: %clang_host %s -fno-standalone-debug -g \
+// RUN: %clang_host %s -fno-standalone-debug -g -std=c99 \
 // RUN:   -gdwarf-5 -gpubnames -gsplit-dwarf -c -o %t2.o -DTWO
-// RUN: %clang_host %t1.o %t2.o -o %t
+// RUN: %clang_host %t1.o %t2.o -o %t -std=c99
 // RUN: %lldb %t -o "log enable 'lldb' object" -o "settings set 
stop-line-count-before 0" \
 // RUN:   -o "b main" -o "run" -o "image lookup -n main -v" -b | FileCheck %s
 



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [lldb] 40ade84 - Revert "Store OptTable::Info::Name as a StringRef"

[via lldb-commits]

Author: serge-sans-paille
Date: 2022-12-07T17:29:53+01:00
New Revision: 40ade845be698355b230abc19c7a76b200188772

URL: 
https://github.com/llvm/llvm-project/commit/40ade845be698355b230abc19c7a76b200188772
DIFF: 
https://github.com/llvm/llvm-project/commit/40ade845be698355b230abc19c7a76b200188772.diff

LOG: Revert "Store OptTable::Info::Name as a StringRef"

Another revert, for another set of issues I don't reproduce locally...

see https://lab.llvm.org/buildbot/#/builders/139/builds/32327

This reverts commit bdfa3100dc3ea9e9ce4d3d4100ea6bb4c3fa2b81.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/unittests/Option/OptionMarshallingTest.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 60d62e2b9c5c1..4621850f13772 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList , const 
ToolChain ) {
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
 const Driver  = TC.getDriver();
 const llvm::opt::OptTable  = D.getOpts();
-StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
-StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
+const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
 D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
   }
   return HasStaticPIE;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index fde098840be4b..9d89148616be1 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1055,7 +1055,7 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   // Only add the version-min options if we got a version from somewhere
   if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
 #define OPTION(PREFIX, NAME, VAR, ...) 
\
-  llvm::StringRef opt_##VAR = NAME;
\
+  const char *opt_##VAR = NAME;
\
   (void)opt_##VAR;
 #include "clang/Driver/Options.inc"
 #undef OPTION

diff  --git a/llvm/include/llvm/Option/OptTable.h 
b/llvm/include/llvm/Option/OptTable.h
index e884ebeb788c4..07d9870f71b33 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -44,7 +44,7 @@ class OptTable {
 /// A null terminated array of prefix strings to apply to name while
 /// matching.
 const char *const *Prefixes;
-StringRef Name;
+const char *Name;
 const char *HelpText;
 const char *MetaVar;
 unsigned ID;
@@ -102,7 +102,9 @@ class OptTable {
   const Option getOption(OptSpecifier Opt) const;
 
   /// Lookup the name of the given option.
-  StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
+  const char *getOptionName(OptSpecifier id) const {
+return getInfo(id).Name;
+  }
 
   /// Get the kind of the given option.
   unsigned getOptionKind(OptSpecifier id) const {
@@ -182,7 +184,7 @@ class OptTable {
   ///  takes
   ///
   /// \return true in success, and false in fail.
-  bool addValues(StringRef Option, const char *Values);
+  bool addValues(const char *Option, const char *Values);
 
   /// Parse a single argument; returning the new argument and
   /// updating Index.

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index 06ed6b78e52f9..ef4873eb7f9c4 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -36,23 +36,31 @@ namespace opt {
 // Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
 // with an exception. '\0' comes at the end of the alphabet instead of the
 // beginning (thus options precede any other options which prefix them).
-static int StrCmpOptionNameIgnoreCase(StringRef A, StringRef B) {
-  size_t MinSize = std::min(A.size(), B.size());
-  if (int Res = A.substr(0, MinSize).compare_insensitive(B.substr(0, MinSize)))
-return Res;
+static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
+  const char *X = A, *Y = B;
+  char a = tolower(*A), b = tolower(*B);
+  while (a == b) {
+if (a == '\0')
+  return 0;
+
+a = tolower(*++X);
+b = tolower(*++Y);
+  }
 
-  if (A.size() == B.size())
-return 0;
+  if (a == '\0') // A is a prefix of B.
+return 1;
+  if (b == '\0') // B is a prefix of A.
+return -1;
 
-  return (A.size() == MinSize) ? 1 /* A is a prefix of B. */
- 

[Lldb-commits] [lldb] bdfa310 - Store OptTable::Info::Name as a StringRef

[via lldb-commits]

Author: serge-sans-paille
Date: 2022-12-07T16:32:37+01:00
New Revision: bdfa3100dc3ea9e9ce4d3d4100ea6bb4c3fa2b81

URL: 
https://github.com/llvm/llvm-project/commit/bdfa3100dc3ea9e9ce4d3d4100ea6bb4c3fa2b81
DIFF: 
https://github.com/llvm/llvm-project/commit/bdfa3100dc3ea9e9ce4d3d4100ea6bb4c3fa2b81.diff

LOG: Store OptTable::Info::Name as a StringRef

This is a recommit of 8ae18303f97d5dcfaecc90b4d87effb2011ed82e,
with a few cleanups.

This avoids implicit conversion to StringRef at several points, which in
turns avoid redundant calls to strlen.

As a side effect, this greatly simplifies the implementation of
StrCmpOptionNameIgnoreCase.

It also eventually gives a consistent, humble speedup in compilation
time (timing updated since original commit).

https://llvm-compile-time-tracker.com/compare.php?from=76fcfea283472a80356d87c89270b0e2d106b54c=b70eb1f347f22fe4d2977360c4ed701eabc43994=instructions:u

Differential Revision: https://reviews.llvm.org/D139274

Added: 


Modified: 
clang/lib/Driver/ToolChains/Gnu.cpp
lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
llvm/include/llvm/Option/OptTable.h
llvm/lib/Option/OptTable.cpp
llvm/unittests/Option/OptionMarshallingTest.cpp
llvm/utils/TableGen/OptParserEmitter.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Gnu.cpp 
b/clang/lib/Driver/ToolChains/Gnu.cpp
index 4621850f13772..60d62e2b9c5c1 100644
--- a/clang/lib/Driver/ToolChains/Gnu.cpp
+++ b/clang/lib/Driver/ToolChains/Gnu.cpp
@@ -331,8 +331,8 @@ static bool getStaticPIE(const ArgList , const 
ToolChain ) {
   if (HasStaticPIE && Args.hasArg(options::OPT_nopie)) {
 const Driver  = TC.getDriver();
 const llvm::opt::OptTable  = D.getOpts();
-const char *StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
-const char *NoPIEName = Opts.getOptionName(options::OPT_nopie);
+StringRef StaticPIEName = Opts.getOptionName(options::OPT_static_pie);
+StringRef NoPIEName = Opts.getOptionName(options::OPT_nopie);
 D.Diag(diag::err_drv_cannot_mix_options) << StaticPIEName << NoPIEName;
   }
   return HasStaticPIE;

diff  --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp 
b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 9d89148616be1..fde098840be4b 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1055,7 +1055,7 @@ void 
PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
   // Only add the version-min options if we got a version from somewhere
   if (!version.empty() && sdk_type != XcodeSDK::Type::Linux) {
 #define OPTION(PREFIX, NAME, VAR, ...) 
\
-  const char *opt_##VAR = NAME;
\
+  llvm::StringRef opt_##VAR = NAME;
\
   (void)opt_##VAR;
 #include "clang/Driver/Options.inc"
 #undef OPTION

diff  --git a/llvm/include/llvm/Option/OptTable.h 
b/llvm/include/llvm/Option/OptTable.h
index 07d9870f71b33..e884ebeb788c4 100644
--- a/llvm/include/llvm/Option/OptTable.h
+++ b/llvm/include/llvm/Option/OptTable.h
@@ -44,7 +44,7 @@ class OptTable {
 /// A null terminated array of prefix strings to apply to name while
 /// matching.
 const char *const *Prefixes;
-const char *Name;
+StringRef Name;
 const char *HelpText;
 const char *MetaVar;
 unsigned ID;
@@ -102,9 +102,7 @@ class OptTable {
   const Option getOption(OptSpecifier Opt) const;
 
   /// Lookup the name of the given option.
-  const char *getOptionName(OptSpecifier id) const {
-return getInfo(id).Name;
-  }
+  StringRef getOptionName(OptSpecifier id) const { return getInfo(id).Name; }
 
   /// Get the kind of the given option.
   unsigned getOptionKind(OptSpecifier id) const {
@@ -184,7 +182,7 @@ class OptTable {
   ///  takes
   ///
   /// \return true in success, and false in fail.
-  bool addValues(const char *Option, const char *Values);
+  bool addValues(StringRef Option, const char *Values);
 
   /// Parse a single argument; returning the new argument and
   /// updating Index.

diff  --git a/llvm/lib/Option/OptTable.cpp b/llvm/lib/Option/OptTable.cpp
index ef4873eb7f9c4..06ed6b78e52f9 100644
--- a/llvm/lib/Option/OptTable.cpp
+++ b/llvm/lib/Option/OptTable.cpp
@@ -36,31 +36,23 @@ namespace opt {
 // Ordering on Info. The ordering is *almost* case-insensitive lexicographic,
 // with an exception. '\0' comes at the end of the alphabet instead of the
 // beginning (thus options precede any other options which prefix them).
-static int StrCmpOptionNameIgnoreCase(const char *A, const char *B) {
-  const char *X = A, *Y = B;
-  char a = tolower(*A), b = tolower(*B);
-  while (a == b) {
-if (a == '\0')
-  return 0;
-
-a = tolower(*++X);
-b = tolower(*++Y);
-  }
+static int StrCmpOptionNameIgnoreCase(StringRef A, 

[Lldb-commits] [PATCH] D139461: Fix dwarf5-lazy-dwo.c for the default c target not being c99.

[Eric Leese via Phabricator via lldb-commits]

Eric accepted this revision.
Eric added a comment.
This revision is now accepted and ready to land.

LGTM. It would also be fine to not check the language.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139461/new/

https://reviews.llvm.org/D139461

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D139484: [lldb/test] Fix data racing issue in TestStackCoreScriptedProcess

[Med Ismail Bennani via Phabricator via lldb-commits]

mib updated this revision to Diff 480796.
mib added a comment.

Address @bulbazord comments


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139484/new/

https://reviews.llvm.org/D139484

Files:
  lldb/test/API/functionalities/scripted_process/Makefile
  lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
  lldb/test/API/functionalities/scripted_process/baz.c
  lldb/test/API/functionalities/scripted_process/baz.cpp
  lldb/test/API/functionalities/scripted_process/baz.h
  lldb/test/API/functionalities/scripted_process/main.cpp

Index: lldb/test/API/functionalities/scripted_process/main.cpp
===
--- lldb/test/API/functionalities/scripted_process/main.cpp
+++ lldb/test/API/functionalities/scripted_process/main.cpp
@@ -1,10 +1,10 @@
-#include 
-#include 
 #include 
 
-extern "C" {
-int baz(int);
-}
+#include "baz.h"
+
+std::condition_variable cv;
+std::mutex mutex;
+bool done;
 
 int bar(int i) {
   int j = i * i;
@@ -13,23 +13,26 @@
 
 int foo(int i) { return bar(i); }
 
-void call_and_wait(int ) {
-  std::cout << "waiting for computation!" << std::endl;
-  while (baz(n) != 42)
-;
-  std::cout << "finished computation!" << std::endl;
+void compute_pow(int ) {
+  std::unique_lock lock(mutex);
+  cv.notify_one();
+  n = foo(n);
+  done = true;
+  cv.notify_one();
 }
 
-void compute_pow(int ) { n = foo(n); }
+void call_and_wait(int , std::mutex , std::condition_variable ) {
+  while (baz(n, mutex, cv, done) != 42) {
+;
+  }
+}
 
 int main() {
   int n = 42;
-  std::mutex mutex;
-  std::unique_lock lock(mutex);
-
-  std::thread thread_1(call_and_wait, std::ref(n));
+  done = false;
+  std::thread thread_1(call_and_wait, std::ref(n), std::ref(mutex),
+   std::ref(cv));
   std::thread thread_2(compute_pow, std::ref(n));
-  lock.unlock();
 
   thread_1.join();
   thread_2.join();
Index: lldb/test/API/functionalities/scripted_process/baz.h
===
--- lldb/test/API/functionalities/scripted_process/baz.h
+++ lldb/test/API/functionalities/scripted_process/baz.h
@@ -1,3 +1,6 @@
 #pragma once
 
-int baz(int j);
+#include 
+#include 
+
+int baz(int j, std::mutex , std::condition_variable , bool );
Index: lldb/test/API/functionalities/scripted_process/baz.cpp
===
--- /dev/null
+++ lldb/test/API/functionalities/scripted_process/baz.cpp
@@ -0,0 +1,10 @@
+#include "baz.h"
+
+#include 
+
+int baz(int j, std::mutex , std::condition_variable , bool ) {
+  std::unique_lock lock(mutex);
+  cv.wait(lock, [] { return done; });
+  int k = sqrt(j);
+  return k; // break here
+}
Index: lldb/test/API/functionalities/scripted_process/baz.c
===
--- lldb/test/API/functionalities/scripted_process/baz.c
+++ /dev/null
@@ -1,8 +0,0 @@
-#include "baz.h"
-
-#include 
-
-int baz(int j) {
-  int k = sqrt(j);
-  return k; // break here
-}
Index: lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
===
--- lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
+++ lldb/test/API/functionalities/scripted_process/TestStackCoreScriptedProcess.py
@@ -17,7 +17,7 @@
 def create_stack_skinny_corefile(self, file):
 self.build()
 target, process, thread, _ = lldbutil.run_to_source_breakpoint(self, "// break here",
-   lldb.SBFileSpec("baz.c"))
+   lldb.SBFileSpec("baz.cpp"))
 self.assertTrue(process.IsValid(), "Process is invalid.")
 # FIXME: Use SBAPI to save the process corefile.
 self.runCmd("process save-core -s stack  " + file)
@@ -109,9 +109,9 @@
 self.assertTrue(func, "Invalid function.")
 
 self.assertIn("baz", frame.GetFunctionName())
-self.assertEqual(frame.vars.GetSize(), 2)
-self.assertEqual(int(frame.vars.GetFirstValueByName('j').GetValue()), 42 * 42)
+self.assertGreater(frame.vars.GetSize(), 0)
 self.assertEqual(int(frame.vars.GetFirstValueByName('k').GetValue()), 42)
+self.assertEqual(int(frame.vars.GetFirstValueByName('j').GetValue()), 42 * 42)
 
 corefile_dylib = self.get_module_with_name(corefile_target, 'libbaz.dylib')
 self.assertTrue(corefile_dylib, "Dynamic library libbaz.dylib not found.")
Index: lldb/test/API/functionalities/scripted_process/Makefile
===
--- lldb/test/API/functionalities/scripted_process/Makefile
+++ lldb/test/API/functionalities/scripted_process/Makefile
@@ -6,8 +6,8 @@
 
 all: libbaz.dylib a.out
 
-libbaz.dylib: baz.c
+libbaz.dylib: baz.cpp
 	$(MAKE) -f $(MAKEFILE_RULES) ARCH=$(ARCH) \
-	

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Tobias Hieta via Phabricator via lldb-commits]

thieta added a comment.

I think this is ready to land @Mordante or is there anything else missing?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D100808: [Propeller] Use Fixed MBB ID instead of volatile MachineBasicBlock::Number.

[Rahman Lavaee via Phabricator via lldb-commits]

rahmanl updated this revision to Diff 479906.
rahmanl added a comment.

- Rebase.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100808/new/

https://reviews.llvm.org/D100808

Files:
  llvm/include/llvm/CodeGen/BasicBlockSectionsProfileReader.h
  llvm/include/llvm/CodeGen/MachineBasicBlock.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/include/llvm/Object/ELFTypes.h
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/BasicBlockSections.cpp
  llvm/lib/CodeGen/BasicBlockSectionsProfileReader.cpp
  llvm/lib/CodeGen/MIRParser/MILexer.cpp
  llvm/lib/CodeGen/MIRParser/MILexer.h
  llvm/lib/CodeGen/MIRParser/MIParser.cpp
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/CodeGen/MachineFunction.cpp
  llvm/lib/Object/ELF.cpp
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/lib/ObjectYAML/ELFYAML.cpp
  llvm/test/CodeGen/X86/basic-block-labels-mir-parse.mir
  llvm/test/CodeGen/X86/basic-block-sections-mir-print.ll
  llvm/test/tools/llvm-readobj/ELF/bb-addr-map.test
  llvm/test/tools/obj2yaml/ELF/bb-addr-map.yaml
  llvm/test/tools/yaml2obj/ELF/bb-addr-map.yaml
  llvm/tools/llvm-readobj/ELFDumper.cpp
  llvm/tools/obj2yaml/elf2yaml.cpp
  llvm/unittests/Object/ELFObjectFileTest.cpp

Index: llvm/unittests/Object/ELFObjectFileTest.cpp
===
--- llvm/unittests/Object/ELFObjectFileTest.cpp
+++ llvm/unittests/Object/ELFObjectFileTest.cpp
@@ -528,7 +528,7 @@
   // Check that we can detect unsupported versions.
   SmallString<128> UnsupportedVersionYamlString(CommonYamlString);
   UnsupportedVersionYamlString += R"(
-Version: 2
+Version: 3
 BBEntries:
   - AddressOffset: 0x0
 Size:  0x1
@@ -536,13 +536,14 @@
 )";
 
   DoCheck(UnsupportedVersionYamlString,
-  "unsupported SHT_LLVM_BB_ADDR_MAP version: 2");
+  "unsupported SHT_LLVM_BB_ADDR_MAP version: 3");
 
   SmallString<128> CommonVersionedYamlString(CommonYamlString);
   CommonVersionedYamlString += R"(
-Version: 1
+Version: 2
 BBEntries:
-  - AddressOffset: 0x0
+  - ID:1
+AddressOffset: 0x0
 Size:  0x1
 Metadata:  0x2
 )";
@@ -551,9 +552,9 @@
   // truncated.
   SmallString<128> TruncatedYamlString(CommonVersionedYamlString);
   TruncatedYamlString += R"(
-ShSize: 0xa
+ShSize: 0xb
 )";
-  DoCheck(TruncatedYamlString, "unable to decode LEB128 at offset 0x000a: "
+  DoCheck(TruncatedYamlString, "unable to decode LEB128 at offset 0x000b: "
"malformed uleb128, extends past end");
 
   // Check that we can detect when the encoded BB entry fields exceed the UINT32
@@ -561,29 +562,32 @@
   SmallVector, 3> OverInt32LimitYamlStrings(
   3, CommonVersionedYamlString);
   OverInt32LimitYamlStrings[0] += R"(
-  - AddressOffset: 0x1
+  - ID:1
+AddressOffset: 0x1
 Size:  0x
 Metadata:  0x
 )";
 
   OverInt32LimitYamlStrings[1] += R"(
-  - AddressOffset: 0x
+  - ID:2
+AddressOffset: 0x
 Size:  0x1
 Metadata:  0x
 )";
 
   OverInt32LimitYamlStrings[2] += R"(
-  - AddressOffset: 0x
+  - ID:3
+AddressOffset: 0x
 Size:  0x
 Metadata:  0x1
 )";
 
   DoCheck(OverInt32LimitYamlStrings[0],
-  "ULEB128 value at offset 0xe exceeds UINT32_MAX (0x1)");
+  "ULEB128 value at offset 0x10 exceeds UINT32_MAX (0x1)");
   DoCheck(OverInt32LimitYamlStrings[1],
-  "ULEB128 value at offset 0x13 exceeds UINT32_MAX (0x1)");
+  "ULEB128 value at offset 0x15 exceeds UINT32_MAX (0x1)");
   DoCheck(OverInt32LimitYamlStrings[2],
-  "ULEB128 value at offset 0x18 exceeds UINT32_MAX (0x1)");
+  "ULEB128 value at offset 0x1a exceeds UINT32_MAX (0x1)");
 
   // Check the proper error handling when the section has fields exceeding
   // UINT32 and is also truncated. This is for checking that we don't generate
@@ -592,24 +596,24 @@
   3, OverInt32LimitYamlStrings[1]);
   // Truncate before the end of the 5-byte field.
   OverInt32LimitAndTruncated[0] += R"(
-ShSize: 0x17
+ShSize: 0x19
 )";
   // Truncate at the end of the 5-byte field.
   OverInt32LimitAndTruncated[1] += R"(
-ShSize: 0x18
+ShSize: 0x1a
 )";
   // Truncate after the end of the 5-byte field.
   OverInt32LimitAndTruncated[2] += R"(
-ShSize: 0x19
+ShSize: 0x1b
 )";
 
   DoCheck(OverInt32LimitAndTruncated[0],
-  "unable to decode LEB128 at offset 0x0013: malformed uleb128, "
+  "unable to decode LEB128 at offset 

[Lldb-commits] [PATCH] D138141: [amdgpu] Reimplement LDS lowering

[Jon Chesterfield via Phabricator via lldb-commits]

JonChesterfield updated this revision to Diff 480484.
JonChesterfield added a comment.
Herald added subscribers: cfe-commits, libc-commits, libcxx-commits, 
openmp-commits, lldb-commits, Sanitizers, hanchung, kadircet, jsetoain, 
Moerafaat, zero9178, pcwang-thead, anlunx, steakhal, mtrofin, Enna1, 
bzcheeseman, mravishankar, mattd, gchakrabarti, ThomasRaoux, pmatos, asb, 
jhenderson, yota9, ayermolo, awarzynski, sdasgup3, asavonic, carlosgalvezp, 
jeroen.dobbelaere, abrachet, wenzhicui, wrengr, armkevincheng, ormris, sjarus, 
eric-k256, mstorsjo, ChuanqiXu, cota, teijeong, frasercrmck, rdzhabarov, 
ecnelises, tatianashp, wenlei, cishida, mehdi_amini, okura, jdoerfert, 
msifontes, sstefan1, jurahul, kuter, cmtice, Kayjukh, vkmr, grosul1, martong, 
Joonsoo, stephenneuendorffer, phosek, liufengdb, aartbik, mgester, 
arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, rriddle, 
luismarques, apazos, sameer.abuasal, pengfei, s.egerton, dmgreen, Jim, 
asbirlea, miyuki, jocewei, PkmX, arphaman, george.burgess.iv, the_o, 
brucehoult, MartinMosbeck, rogfer01, steven_wu, atanasyan, edward-jones, 
zzheng, MaskRay, jrtc27, gbedwell, niosHD, sabuasal, simoncook, haicheng, 
johnrusso, rbar, fedor.sergeev, kbarton, aheejin, jgravelle-google, 
krytarowski, arichardson, sbc100, nemanjai, sdardis, emaste, dylanmckay, 
jyknight, dschuff, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: andreadb.
Herald added a reviewer: shafik.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jhenderson.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: awarzynski.
Herald added a reviewer: mravishankar.
Herald added a reviewer: bondhugula.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: paulkirth.
Herald added a reviewer: NoQ.
Herald added a reviewer: ributzka.
Herald added a reviewer: dcaballe.
Herald added a reviewer: njames93.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc-project, 
libc++abi, MLIR, lld-macho, clang-tools-extra, Flang.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: lld-macho.
This revision now requires review to proceed.

- remove obsolete comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138141/new/

https://reviews.llvm.org/D138141

Files:
  bolt/include/bolt/Core/BinaryBasicBlock.h
  bolt/include/bolt/Core/BinaryFunction.h
  bolt/include/bolt/Core/BinarySection.h
  bolt/lib/Core/BinaryEmitter.cpp
  bolt/lib/Core/BinarySection.cpp
  bolt/lib/Profile/DataAggregator.cpp
  bolt/lib/Profile/YAMLProfileWriter.cpp
  clang-tools-extra/clang-tidy/modernize/UseNullptrCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/TidyFastChecks.inc
  clang-tools-extra/clangd/TidyFastChecks.py
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/support/ThreadsafeFS.h
  clang-tools-extra/clangd/tool/Check.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/include-cleaner/tool/IncludeCleaner.cpp
  clang-tools-extra/include-cleaner/unittests/RecordTest.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-nullptr-cxx20.cpp
  clang/cmake/modules/ClangConfig.cmake.in
  clang/docs/ReleaseNotes.rst
  clang/docs/tools/clang-formatted-files.txt
  clang/include/clang/AST/Decl.h
  clang/include/clang/Analysis/Analyses/PostOrderCFGView.h
  clang/include/clang/Basic/BuiltinsAMDGPU.def
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/StaticAnalyzer/Checkers/LocalCheckers.h
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
  clang/include/clang/Tooling/Inclusions/HeaderAnalysis.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprConstant.cpp
  

[Lldb-commits] [PATCH] D100808: [Propeller] Use Fixed MBB ID instead of volatile MachineBasicBlock::Number.

[Rahman Lavaee via Phabricator via lldb-commits]

rahmanl updated this revision to Diff 479905.
rahmanl added a comment.
Herald added subscribers: cfe-commits, libc-commits, openmp-commits, 
libcxx-commits, lldb-commits, Sanitizers, hanchung, kadircet, jsetoain, 
Moerafaat, zero9178, pcwang-thead, anlunx, steakhal, mtrofin, Enna1, 
bzcheeseman, kosarev, mravishankar, mattd, gchakrabarti, ThomasRaoux, pmatos, 
asb, yota9, ayermolo, awarzynski, arjunp, sdasgup3, asavonic, carlosgalvezp, 
jeroen.dobbelaere, abrachet, Groverkss, wenzhicui, wrengr, armkevincheng, 
ormris, foad, sjarus, eric-k256, ChuanqiXu, cota, teijeong, frasercrmck, 
rdzhabarov, ecnelises, tatianashp, wenlei, mehdi_amini, okura, jdoerfert, 
bmahjour, msifontes, sstefan1, jurahul, kuter, cmtice, Kayjukh, lebedev.ri, 
vkmr, grosul1, martong, Joonsoo, stephenneuendorffer, phosek, kerbowa, 
liufengdb, aartbik, mgester, arpith-jacob, csigg, nicolasvasilache, 
antiagainst, shauheen, rriddle, luismarques, apazos, sameer.abuasal, s.egerton, 
dmgreen, Jim, asbirlea, thopre, jocewei, PkmX, arphaman, george.burgess.iv, 
the_o, brucehoult, MartinMosbeck, rogfer01, steven_wu, atanasyan, mgrang, 
edward-jones, zzheng, jrtc27, gbedwell, niosHD, cryptoad, sabuasal, simoncook, 
haicheng, johnrusso, rbar, javed.absar, fedor.sergeev, kbarton, aheejin, 
jgravelle-google, arichardson, sbc100, jvesely, nemanjai, sdardis, dylanmckay, 
jyknight, dschuff, arsenm, qcolombet, MatzeB, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: JDevlieghere.
Herald added a reviewer: andreadb.
Herald added a reviewer: shafik.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: rriddle.
Herald added a reviewer: antiagainst.
Herald added a reviewer: aartbik.
Herald added a reviewer: MaskRay.
Herald added a reviewer: sscalpone.
Herald added a reviewer: lebedev.ri.
Herald added a reviewer: ftynse.
Herald added a reviewer: aaron.ballman.
Herald added a reviewer: aartbik.
Herald added a reviewer: awarzynski.
Herald added a reviewer: mravishankar.
Herald added a reviewer: clementval.
Herald added a reviewer: bondhugula.
Herald added a reviewer: zuban32.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: dang.
Herald added a reviewer: ThomasRaoux.
Herald added a reviewer: NoQ.
Herald added a reviewer: ributzka.
Herald added a reviewer: dcaballe.
Herald added a reviewer: njames93.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc-project, 
MLIR, lld-macho, clang-tools-extra, Flang.
Herald added a reviewer: libc++.
Herald added a reviewer: lld-macho.

- Changed the emitted BB address map version back to 1.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100808/new/

https://reviews.llvm.org/D100808

Files:
  .mailmap
  bolt/include/bolt/Core/BinaryContext.h
  bolt/include/bolt/Core/BinaryFunction.h
  bolt/include/bolt/Core/DebugData.h
  bolt/include/bolt/Core/MCPlusBuilder.h
  bolt/include/bolt/Passes/ReachingDefOrUse.h
  bolt/include/bolt/Rewrite/DWARFRewriter.h
  bolt/lib/Core/BinaryContext.cpp
  bolt/lib/Core/DebugData.cpp
  bolt/lib/Core/MCPlusBuilder.cpp
  bolt/lib/Passes/AsmDump.cpp
  bolt/lib/Passes/DataflowInfoManager.cpp
  bolt/lib/Profile/BoltAddressTranslation.cpp
  bolt/lib/Profile/DataAggregator.cpp
  bolt/lib/Profile/DataReader.cpp
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/lib/Rewrite/MachORewriteInstance.cpp
  bolt/lib/Rewrite/RewriteInstance.cpp
  bolt/lib/Target/X86/X86MCPlusBuilder.cpp
  bolt/lib/Utils/Utils.cpp
  clang-tools-extra/clang-doc/HTMLGenerator.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllMacros.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/clang-query/Query.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/ClangTidyCheck.h
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyOptions.cpp
  clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
  clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
  clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
  clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Fangrui Song via Phabricator via lldb-commits]

MaskRay accepted this revision.
MaskRay added a comment.

Please check these builds all work:

- default
- `-DLLVM_LINK_LLVM_DYLIB=on`
- `-DBUILD_SHARED_LIBS=on`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] green dragon testsuite crash on TestCoroutineHandle.py from coroutines formatter changes, temporarily revert?

[Adrian Vogelsgesang via lldb-commits]

> I don't know what tools are installed on the builder but I'll ask around
the office on Monday, I know there are people who are more familiar with
how these bots are set up.

Sounds good. Thank you!

> FWIW I could repo promise_type failure on my macOS desktop which has the
current Xcode 14 tools installed

You mean you were able to reproduce the crash? Or were you able to
reproduce the test failure described in
https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189
also on your macOS desktop?

My current hypothesis is:

   1.
   
https://github.com/llvm/llvm-project/commit/4346318f5c700f4e85f866610fb8328fc429319b
   failed because
   - green dragon is using an outdated pre-release version of clang
  - as commented in the test case "clang version < 16 did not yet write
  debug info for the noop coroutines."
  - the test case hence uses the condition "if not (is_clang and
  self.expectedCompilerVersion(["<", "16"])):"" to only expect check this
  test expectation on recent enough compilers
  - my understanding is that "clang-16 development builds" would pass
  this version check, even if they did not yet include
  https://reviews.llvm.org/D132580 and then fail the test expectation
  - If it turns out to be true that green dragon uses an older
  clang-version, I would propose the following course of action:
 - Somebody should upgrades the version of clang used in green
 dragon
 - I resubmit this change unchanged
  2.
   
https://github.com/llvm/llvm-project/commit/cd3091a88f7c55c90d9b5fff372ce1cdfc71948d
   - was indeed buggy. This commit turned a "missing debug information"
  situation into a crash
  - I am pretty sure that Apple clang version 14.0.0 did not contain
  https://reviews.llvm.org/D132580, and is hence missing the debug
  information. This would explain the crash on your mac Desktop.
  - I would first wait after we resolved the issues around the first
  commit, though, because rebasing this commit so it can be applied before
  4346318f5c700f4e85f866610fb8328fc429319b would be cumbersome
  - I will then add the additional `if (!promise_type.isVoid())` check,
  so we no longer crash on missing debug info, and then re-apply the patch

Do you agree with that plan? Do you know somebody who can update clang on
green dragon?

Cheers,
Adrian

On Sat, Nov 26, 2022 at 3:24 AM Jason Molenda  wrote:

> FWIW I could repo promise_type failure on my macOS desktop which has the
> current Xcode 14 tools installed, but I can't tell you exactly what this
> corresponds to in github hash terms (it's "Apple clang version 14.0.0
> (clang-1400.0.29.100)"), I think it was originally branched 2021-10-26,
> although there have been a lot of cherrypicks pulled in too, so it's not an
> exact snapshot of October 26 sources.
>
> J
>
> > On Nov 25, 2022, at 2:22 PM, Adrian Vogelsgesang <
> avogelsges...@salesforce.com> wrote:
> >
> > Hm... wait a second...
> >
> > Which clang-version is
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ using? Is it
> using an older version of clang? Does that version already contain the
> patch https://reviews.llvm.org/D132580?
> >
> > If not, that might explain things...
> >
> > On Fri, Nov 25, 2022 at 11:18 PM Adrian Vogelsgesang <
> avogelsges...@salesforce.com> wrote:
> > Hi Jason,
> >
> > Sorry for the late reply - your mail got caught in my spam filter, and I
> just realized about the build breakage now after you reverted the commits.
> Just to confirm:
> >
> > I looked at the original change again, and I think I know what's going
> wrong here: Devirtualization fails, and `promise_type` stays `void`.
> Obviously, `CreateValueObjectFromAddress` cannot create an object of type
> `void` and fails. The previous version was robust against that scenario,
> but https://reviews.llvm.org/D132815 accidentally regressed this.
> >
> > If a program has all the expected debug info, devirtualization should
> never fail and `promise_type` never stays `void`. It seems something is
> wrong/unexpected with the debug info on Darwin. Devirtualization relies on
> being able to identify the function pointer of the `destroy` function and
> inspects that function. So the root cause seems to be the same as for the
> revert
> https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189:
> For some reason, the debugger cannot find debug info for the function
> pointed to by the `destroy` function.
> >
> > However, I still don't quite understand what's wrong with the debug info
> on Mac.
> >
> > Either way, the pretty printer should of course be robust against such
> unexpected debug info. I think a simple additional check should be enough
> to make this robust so we no longer crash:
> > if (!promise_type.isVoid()) { // < additional check
> >   lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
> >   "promise", frame_ptr_addr + 2 * 

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Francesco Petrogalli via Phabricator via lldb-commits]

fpetrogalli accepted this revision.
fpetrogalli added a comment.
This revision is now accepted and ready to land.

Thank you for working on this @lenary  - LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] green dragon testsuite crash on TestCoroutineHandle.py from coroutines formatter changes, temporarily revert?

[Adrian Vogelsgesang via lldb-commits]

Hm... wait a second...

Which clang-version is
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/ using? Is it
using an older version of clang? Does that version already contain the
patch https://reviews.llvm.org/D132580?

If not, that might explain things...

On Fri, Nov 25, 2022 at 11:18 PM Adrian Vogelsgesang <
avogelsges...@salesforce.com> wrote:

> Hi Jason,
>
> Sorry for the late reply - your mail got caught in my spam filter, and I
> just realized about the build breakage now after you reverted the commits.
> Just to confirm:
>
> I looked at the original change again, and I think I know what's going
> wrong here: Devirtualization fails, and `promise_type` stays `void`.
> Obviously, `CreateValueObjectFromAddress` cannot create an object of type
> `void` and fails. The previous version was robust against that scenario,
> but https://reviews.llvm.org/D132815 accidentally regressed this.
>
> If a program has all the expected debug info, devirtualization should
> never fail and `promise_type` never stays `void`. It seems something is
> wrong/unexpected with the debug info on Darwin. Devirtualization relies on
> being able to identify the function pointer of the `destroy` function and
> inspects that function. So the root cause seems to be the same as for the
> revert
> https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189:
> For some reason, the debugger cannot find debug info for the function
> pointed to by the `destroy` function.
>
> However, I still don't quite understand what's wrong with the debug info
> on Mac.
>
> Either way, the pretty printer should of course be robust against such
> unexpected debug info. I think a simple additional check should be enough
> to make this robust so we no longer crash:
> if (!promise_type.isVoid()) { // < additional check
>   lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
>   "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
>   Status error;
>   lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
>   if (error.Success())
> m_promise_ptr_sp = promisePtr->Clone(ConstString("promise"));
> }
>
> Meta question: I am a bit confused about
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/. I thought
> LLVM's buildbots were set up to send an email in case somebody breaks the
> build. Also, I made sure that all builds in
> https://lab.llvm.org/buildbot/#/builders stayed green after my commit. So
> what are the expectations around which CIs need to stay green after a
> commit? Do I need to setup anything such that green.lab.llvm.org also
> sends me an email if I break it?
>
> Cheers,
> Adrian
>
> On Thu, Nov 24, 2022 at 10:50 PM Jason Molenda  wrote:
>
>> Ah, little misstatement below.  My first thought was that promise_type
>> was null, but when I stepped through here with a debugger, it was not.  I
>> didn't know how to dig in to a CompilerType object but there was some
>> reason why this CreateValueObjectFromAddress failed to return an actual
>> ValueObject, and I assumed it's something to do with that CompilerType.
>> But I didn't dig in far enough to understand what the issue in the type was.
>>
>> > On Nov 24, 2022, at 1:20 PM, Jason Molenda  wrote:
>> >
>> > Hi Adrian, the green dragon Incremental CI bot has been failing the
>> past couple of days after the changes for the coroutines formatter, most
>> directly   https://reviews.llvm.org/D132815 landed -
>> TestCoroutineHandle.py results in a segfault on Darwin systems (both on the
>> CI and on my mac desktop) consistently.
>> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
>> >
>> > It's crashing in formatters::StdlibCoroutineHandleSyntheticFrontEnd
>> where you're doing
>> >
>> > ```
>> >  // Add the `promise` member. We intentionally add `promise` as a
>> pointer type
>> >  // instead of a value type, and don't automatically dereference this
>> pointer.
>> >  // We do so to avoid potential very deep recursion in case there is a
>> cycle in
>> >  // formed between `std::coroutine_handle`s and their promises.
>> >  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
>> >  "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
>> >  Status error;
>> >  lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
>> > ```
>> >
>> > and the promise_type dose not have a CompilerType so promisePtr is
>> nullptr and we crash on the method call to AddressOf. I looked briefly, but
>> this isn't a part of lldb I know well and I'm not sure the nature of the
>> problem.
>> >
>> > It's a long weekend in the US, so if you have a suggestion for
>> addressing this that'd be great, or I can roll back the changes necessary
>> to get the bot clean later today and we can look at this next week.
>> >
>> > Thanks!
>>
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

The name `llvm/lib/TargetParser` LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] green dragon testsuite crash on TestCoroutineHandle.py from coroutines formatter changes, temporarily revert?

[Adrian Vogelsgesang via lldb-commits]

Hi Jason,

Sorry for the late reply - your mail got caught in my spam filter, and I
just realized about the build breakage now after you reverted the commits.
Just to confirm:

I looked at the original change again, and I think I know what's going
wrong here: Devirtualization fails, and `promise_type` stays `void`.
Obviously, `CreateValueObjectFromAddress` cannot create an object of type
`void` and fails. The previous version was robust against that scenario,
but https://reviews.llvm.org/D132815 accidentally regressed this.

If a program has all the expected debug info, devirtualization should never
fail and `promise_type` never stays `void`. It seems something is
wrong/unexpected with the debug info on Darwin. Devirtualization relies on
being able to identify the function pointer of the `destroy` function and
inspects that function. So the root cause seems to be the same as for the
revert
https://github.com/llvm/llvm-project/commit/2b2f2f66141d52dc0d3082ddd12805d36872a189:
For some reason, the debugger cannot find debug info for the function
pointed to by the `destroy` function.

However, I still don't quite understand what's wrong with the debug info on
Mac.

Either way, the pretty printer should of course be robust against such
unexpected debug info. I think a simple additional check should be enough
to make this robust so we no longer crash:
if (!promise_type.isVoid()) { // < additional check
  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
  "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
  Status error;
  lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
  if (error.Success())
m_promise_ptr_sp = promisePtr->Clone(ConstString("promise"));
}

Meta question: I am a bit confused about
https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/. I thought
LLVM's buildbots were set up to send an email in case somebody breaks the
build. Also, I made sure that all builds in
https://lab.llvm.org/buildbot/#/builders stayed green after my commit. So
what are the expectations around which CIs need to stay green after a
commit? Do I need to setup anything such that green.lab.llvm.org also sends
me an email if I break it?

Cheers,
Adrian

On Thu, Nov 24, 2022 at 10:50 PM Jason Molenda  wrote:

> Ah, little misstatement below.  My first thought was that promise_type was
> null, but when I stepped through here with a debugger, it was not.  I
> didn't know how to dig in to a CompilerType object but there was some
> reason why this CreateValueObjectFromAddress failed to return an actual
> ValueObject, and I assumed it's something to do with that CompilerType.
> But I didn't dig in far enough to understand what the issue in the type was.
>
> > On Nov 24, 2022, at 1:20 PM, Jason Molenda  wrote:
> >
> > Hi Adrian, the green dragon Incremental CI bot has been failing the past
> couple of days after the changes for the coroutines formatter, most
> directly   https://reviews.llvm.org/D132815 landed -
> TestCoroutineHandle.py results in a segfault on Darwin systems (both on the
> CI and on my mac desktop) consistently.
> https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/
> >
> > It's crashing in formatters::StdlibCoroutineHandleSyntheticFrontEnd
> where you're doing
> >
> > ```
> >  // Add the `promise` member. We intentionally add `promise` as a
> pointer type
> >  // instead of a value type, and don't automatically dereference this
> pointer.
> >  // We do so to avoid potential very deep recursion in case there is a
> cycle in
> >  // formed between `std::coroutine_handle`s and their promises.
> >  lldb::ValueObjectSP promise = CreateValueObjectFromAddress(
> >  "promise", frame_ptr_addr + 2 * ptr_size, exe_ctx, promise_type);
> >  Status error;
> >  lldb::ValueObjectSP promisePtr = promise->AddressOf(error);
> > ```
> >
> > and the promise_type dose not have a CompilerType so promisePtr is
> nullptr and we crash on the method call to AddressOf. I looked briefly, but
> this isn't a part of lldb I know well and I'm not sure the nature of the
> problem.
> >
> > It's a long weekend in the US, so if you have a suggestion for
> addressing this that'd be great, or I can roll back the changes necessary
> to get the bot clean later today and we can look at this next week.
> >
> > Thanks!
>
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via lldb-commits]

lenary added a comment.

> As for the name TargetParser. @arsenm came up with the name 
> SubtargetRegistry. I am fine with either names.

@fpetrogalli I'm going to stick with TargetParser, because I think this is less 
confusing, given that TargetRegistry is already a component.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via lldb-commits]

lenary added inline comments.



Comment at: clang/lib/Lex/CMakeLists.txt:3
 
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  Support

fpetrogalli wrote:
> I wonder how `support` was not being detected as a linking error...
Not sure, but I still think getting the case right is useful in this patch.



Comment at: flang/tools/bbc/CMakeLists.txt:2-4
+  Passes
+  TargetParser
+  )

fpetrogalli wrote:
> nit: extra tabs
I've undone the re-indent here.



Comment at: flang/unittests/Optimizer/CMakeLists.txt:14
   ${dialect_libs}
 )
 

fpetrogalli wrote:
> Why not add `LLVMTargetParser` here?
That's better, done.



Comment at: llvm/include/llvm/ADT/Triple.h:1
-//===-- llvm/ADT/Triple.h - Target triple helper class --*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_ADT_TRIPLE_H
-#define LLVM_ADT_TRIPLE_H
-
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/VersionTuple.h"
-
-// Some system headers or GCC predefined macros conflict with identifiers in
-// this file.  Undefine them here.
-#undef NetBSD
-#undef mips
-#undef sparc
-
-namespace llvm {
-
-/// Triple - Helper class for working with autoconf configuration names. For
-/// historical reasons, we also call these 'triples' (they used to contain
-/// exactly three fields).
-///
-/// Configuration names are strings in the canonical form:
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM
-/// or
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM-ENVIRONMENT
-///
-/// This class is used for clients which want to support arbitrary
-/// configuration names, but also want to implement certain special
-/// behavior for particular configurations. This class isolates the mapping
-/// from the components of the configuration name to well known IDs.
-///
-/// At its core the Triple class is designed to be a wrapper for a triple
-/// string; the constructor does not change or normalize the triple string.
-/// Clients that need to handle the non-canonical triples that users often
-/// specify should use the normalize method.
-///
-/// See autoconf/config.guess for a glimpse into what configuration names
-/// look like in practice.
-class Triple {
-public:
-  enum ArchType {
-UnknownArch,
-
-arm,// ARM (little endian): arm, armv.*, xscale
-armeb,  // ARM (big endian): armeb
-aarch64,// AArch64 (little endian): aarch64
-aarch64_be, // AArch64 (big endian): aarch64_be
-aarch64_32, // AArch64 (little endian) ILP32: aarch64_32
-arc,// ARC: Synopsys ARC
-avr,// AVR: Atmel AVR microcontroller
-bpfel,  // eBPF or extended BPF or 64-bit BPF (little endian)
-bpfeb,  // eBPF or extended BPF or 64-bit BPF (big endian)
-csky,   // CSKY: csky
-dxil,   // DXIL 32-bit DirectX bytecode
-hexagon,// Hexagon: hexagon
-loongarch32,// LoongArch (32-bit): loongarch32
-loongarch64,// LoongArch (64-bit): loongarch64
-m68k,   // M68k: Motorola 680x0 family
-mips,   // MIPS: mips, mipsallegrex, mipsr6
-mipsel, // MIPSEL: mipsel, mipsallegrexe, mipsr6el
-mips64, // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
-mips64el,   // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
-msp430, // MSP430: msp430
-ppc,// PPC: powerpc
-ppcle,  // PPCLE: powerpc (little endian)
-ppc64,  // PPC64: powerpc64, ppu
-ppc64le,// PPC64LE: powerpc64le
-r600,   // R600: AMD GPUs HD2XXX - HD6XXX
-amdgcn, // AMDGCN: AMD GCN GPUs
-riscv32,// RISC-V (32-bit): riscv32
-riscv64,// RISC-V (64-bit): riscv64
-sparc,  // Sparc: sparc
-sparcv9,// Sparcv9: Sparcv9
-sparcel,// Sparc: (endianness = little). NB: 'Sparcle' is a CPU 
variant
-systemz,// SystemZ: s390x
-tce,// TCE (http://tce.cs.tut.fi/): tce
-tcele,  // TCE little endian (http://tce.cs.tut.fi/): tcele
-thumb,  // Thumb (little endian): thumb, thumbv.*
-thumbeb,// Thumb (big endian): thumbeb
-x86,// X86: i[3-9]86
-x86_64, // X86-64: amd64, x86_64
-xcore,  // XCore: xcore
-nvptx,  // NVPTX: 32-bit
-nvptx64,// NVPTX: 64-bit
-le32,   // le32: generic little-endian 32-bit CPU (PNaCl)
-le64,   // le64: generic little-endian 64-bit CPU (PNaCl)
-amdil,  // AMDIL
-amdil64,// AMDIL with 64-bit pointers
-hsail,  // AMD HSAIL
-

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Francesco Petrogalli via Phabricator via lldb-commits]

fpetrogalli added a comment.

@lenary - thank you for the update!

I have added a bunch of miso comments, a bit repetitive... by the time I 
realised they were repetitive it was faster to get to the bottom of it than 
removing them!

In D137838#3931295 , @lenary wrote:

> I've updated the patch to use forwarding headers (and to rebase past some 
> changes that have happened in the interim).
>
> The patch is still huge because of the number of places using the 
> TargetParsers, which need the component added to their CMakeLists.txt.
>
> I think after the next release, I will add `#warning` to the forwarding 
> headers, so their usage can be noticed and updated.

Why not add the `#warning` directive now?

Thank you again,

Francesco




Comment at: clang/lib/Lex/CMakeLists.txt:3
 
-set(LLVM_LINK_COMPONENTS support)
+set(LLVM_LINK_COMPONENTS
+  Support

I wonder how `support` was not being detected as a linking error...



Comment at: clang/lib/Tooling/DumpTool/CMakeLists.txt:5
   ClangSrcLocDump.cpp
-)
+  )
 

nit: unrelated change.



Comment at: flang/tools/bbc/CMakeLists.txt:2-4
+  Passes
+  TargetParser
+  )

nit: extra tabs



Comment at: flang/unittests/Optimizer/CMakeLists.txt:14
   ${dialect_libs}
 )
 

Why not add `LLVMTargetParser` here?



Comment at: llvm/include/llvm/ADT/Triple.h:1
-//===-- llvm/ADT/Triple.h - Target triple helper class --*- C++ 
-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#ifndef LLVM_ADT_TRIPLE_H
-#define LLVM_ADT_TRIPLE_H
-
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/VersionTuple.h"
-
-// Some system headers or GCC predefined macros conflict with identifiers in
-// this file.  Undefine them here.
-#undef NetBSD
-#undef mips
-#undef sparc
-
-namespace llvm {
-
-/// Triple - Helper class for working with autoconf configuration names. For
-/// historical reasons, we also call these 'triples' (they used to contain
-/// exactly three fields).
-///
-/// Configuration names are strings in the canonical form:
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM
-/// or
-///   ARCHITECTURE-VENDOR-OPERATING_SYSTEM-ENVIRONMENT
-///
-/// This class is used for clients which want to support arbitrary
-/// configuration names, but also want to implement certain special
-/// behavior for particular configurations. This class isolates the mapping
-/// from the components of the configuration name to well known IDs.
-///
-/// At its core the Triple class is designed to be a wrapper for a triple
-/// string; the constructor does not change or normalize the triple string.
-/// Clients that need to handle the non-canonical triples that users often
-/// specify should use the normalize method.
-///
-/// See autoconf/config.guess for a glimpse into what configuration names
-/// look like in practice.
-class Triple {
-public:
-  enum ArchType {
-UnknownArch,
-
-arm,// ARM (little endian): arm, armv.*, xscale
-armeb,  // ARM (big endian): armeb
-aarch64,// AArch64 (little endian): aarch64
-aarch64_be, // AArch64 (big endian): aarch64_be
-aarch64_32, // AArch64 (little endian) ILP32: aarch64_32
-arc,// ARC: Synopsys ARC
-avr,// AVR: Atmel AVR microcontroller
-bpfel,  // eBPF or extended BPF or 64-bit BPF (little endian)
-bpfeb,  // eBPF or extended BPF or 64-bit BPF (big endian)
-csky,   // CSKY: csky
-dxil,   // DXIL 32-bit DirectX bytecode
-hexagon,// Hexagon: hexagon
-loongarch32,// LoongArch (32-bit): loongarch32
-loongarch64,// LoongArch (64-bit): loongarch64
-m68k,   // M68k: Motorola 680x0 family
-mips,   // MIPS: mips, mipsallegrex, mipsr6
-mipsel, // MIPSEL: mipsel, mipsallegrexe, mipsr6el
-mips64, // MIPS64: mips64, mips64r6, mipsn32, mipsn32r6
-mips64el,   // MIPS64EL: mips64el, mips64r6el, mipsn32el, mipsn32r6el
-msp430, // MSP430: msp430
-ppc,// PPC: powerpc
-ppcle,  // PPCLE: powerpc (little endian)
-ppc64,  // PPC64: powerpc64, ppu
-ppc64le,// PPC64LE: powerpc64le
-r600,   // R600: AMD GPUs HD2XXX - HD6XXX
-amdgcn, // AMDGCN: AMD GCN GPUs
-riscv32,// RISC-V (32-bit): riscv32
-riscv64,// RISC-V (64-bit): riscv64
-sparc,  // Sparc: sparc
-sparcv9,// Sparcv9: Sparcv9
-sparcel,// Sparc: (endianness = little). NB: 'Sparcle' is a CPU 
variant
-systemz,// SystemZ: 

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via lldb-commits]

lenary added a comment.

I've updated the patch to use forwarding headers (and to rebase past some 
changes that have happened in the interim).

The patch is still huge because of the number of places using the 
TargetParsers, which need the component added to their CMakeLists.txt.

I think after the next release, I will add `#warning` to the forwarding 
headers, so their usage can be noticed and updated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

Moving target-specific parsers outside LLVMSupport LGTM. I even objected a bit 
when more stuff of this kind was introduced into LLVMSupport.

+1 for adding temporary forwarding headers for now to avoid update all 
`#include` users.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via lldb-commits]

lenary added a comment.

In D137838#3921828 , @thakis wrote:

> This is a gigantic diff. I'd recommend keeping the .h files in the old place 
> for v0 and make them just forwarding headers that include the header at the 
> new location. That way, you don't have to update oodles of include lines in 
> this patch, and it makes it a bit easier to see what's going on. (You can 
> then update all the include lines in a trivial follow-up if this change goes 
> through, and then remove the forwarding headers in Support, to cut the 
> dependency you want to remove.)

Thanks for the suggestion. I do agree this patch is too big to land as-is. I 
think this patch is useful as "this shows the full effect of this change", even 
if we find ways to make how we land this patch less invasive. One other thought 
I had was to move the unittests first, but that doesn't make as big a 
difference as the fact that `llvm/ADT/Triple.h` is referenced everywhere.

I would like more comments (either here or on Discourse, link in a prior 
comment) on whether this split is reasonable before I post a v2 of this patch, 
though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Francesco Petrogalli via Phabricator via lldb-commits]

fpetrogalli added a comment.

Hi @lenary  - thank you for working on this!

The patch is reasonable to me.

I agree on the suggestion of using forwarding headers or the first iteration of 
the change, it will make it easier to review.

I'll adapt the auto-get patch at D137517  on 
D137838 .

As for the name `TargetParser`. @arsenm came up with the name 
`SubtargetRegistry`. I am fine with either names.

Francesco


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Nico Weber via Phabricator via lldb-commits]

thakis added a comment.

This is a gigantic diff. I'd recommend keeping the .h files in the old place 
for v0 and make them just forwarding headers that include the header at the new 
location. That way, you don't have to update oodles of include lines in this 
patch, and it makes it a bit easier to see what's going on. (You can then 
update all the include lines in a trivial follow-up if this change goes 
through, and then remove the forwarding headers in Support, to cut the 
dependency you want to remove.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[serge via Phabricator via lldb-commits]

serge-sans-paille added inline comments.



Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {

Ericson2314 wrote:
> mgorny wrote:
> > Well, one thing I immediately notice is that technically 
> > `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations you 
> > are assuming it will always be relative. Not saying that's a blocker but I 
> > think you should add checks for that into `CMakeLists.txt`.
> Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` with an 
> absolute path.
> 
> OK if this isn't supported initially but we should ovoid regressing where 
> possible.
Turns out LLVM_LIBDIR_SUFFIX is used in several situations: (a) for the library 
install dir or (b) for the location where build artifact as stored in 
CMAKE_BINARY_DIR. (a) could accept a path but not (b), unless we derive it from 
(a) but I can see a lot of complication there for the testing step. Would that 
be ok to choke on CMAKE_INSTALL_LIBDIR being a path and not a directory 
component?



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

I landed the Clang bits in 94738a5ac34283bb034b022602b9f9e93d67081f 
, thank 
you for the cleanup!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Nico Weber via Phabricator via lldb-commits]

thakis added inline comments.
Herald added a reviewer: njames93.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:47
   // complete architecture list, nor a reasonable subset. The problem is that
-  // historically the driver driver accepts this and also ties its -march=
   // handling to the architecture name, so we need to be careful before 
removing

aaron.ballman wrote:
> This is duplicated just often enough and in just enough contexts where I 
> think "driver driver" means "the driver that drives another driver", but it'd 
> be nice if someone else could confirm or deny that.
I think that's correct. IIRC, the "driver driver" was a feature in apple gcc 
long ago, and it was a driver that invoked the gcc driver once per `-arch` or 
some such. One of the early goals of clang was to just have a driver, but no 
driver driver.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Kadir Cetinkaya via Phabricator via lldb-commits]

kadircet added a comment.

LGTM for changes under `clang-tools-extra/clangd`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Sam James via Phabricator via lldb-commits]

thesamesam added a comment.

Thanks! Agreed -- I'll file the backport issue now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Guillot Tony via Phabricator via lldb-commits]

to268 accepted this revision.
to268 added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137838: [RFC][Support] Move TargetParsers to new component

[Sam Elliott via Phabricator via lldb-commits]

lenary added a comment.
Herald added a subscriber: Michael137.

Apologies to everyone who has automatically been marked as a reviewer for this 
change. There is more context for it here: 
https://discourse.llvm.org/t/targetparser-auto-generation-of-riscv-cpu-definitions/66419/4?u=lenary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137838/new/

https://reviews.llvm.org/D137838

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Petr Hosek via Phabricator via lldb-commits]

phosek accepted this revision.
phosek added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: clang/CMakeLists.txt:14
   set(CLANG_BUILT_STANDALONE TRUE)
+  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
+message(WARNING

I wonder if we could move this to `CMakePolicy.cmake`, though admittedly you'd 
have to somehow avoid warning multiple times in in-tree builds.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Chuanqi Xu via Phabricator via lldb-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Louis Dionne via Phabricator via lldb-commits]

ldionne added a comment.

LGTM (sorry, it looks like I approved on behalf of all libc++ vendors but that 
wasn't my intention).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131919: Move googletest to the third-party directory

[Tom Stellard via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG59052468c3e3: Move googletest to the third-party directory 
(authored by tstellar).

Changed prior to commit:
  https://reviews.llvm.org/D131919?vs=468784=474310#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131919/new/

https://reviews.llvm.org/D131919

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/utils/unittest/CMakeLists.txt
  llvm/utils/unittest/UnitTestMain/CMakeLists.txt
  llvm/utils/unittest/UnitTestMain/TestMain.cpp
  llvm/utils/unittest/googlemock/LICENSE.txt
  llvm/utils/unittest/googlemock/README.LLVM
  llvm/utils/unittest/googlemock/include/gmock/gmock-actions.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-cardinalities.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-function-mocker.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-generated-actions.h
  
llvm/utils/unittest/googlemock/include/gmock/gmock-generated-function-mockers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-generated-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-more-actions.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-more-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-nice-strict.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-spec-builders.h
  llvm/utils/unittest/googlemock/include/gmock/gmock.h
  
llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-generated-actions.h
  llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-port.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-port.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-pp.h
  llvm/utils/unittest/googlemock/src/gmock-all.cc
  llvm/utils/unittest/googlemock/src/gmock-cardinalities.cc
  llvm/utils/unittest/googlemock/src/gmock-internal-utils.cc
  llvm/utils/unittest/googlemock/src/gmock-matchers.cc
  llvm/utils/unittest/googlemock/src/gmock-spec-builders.cc
  llvm/utils/unittest/googlemock/src/gmock.cc
  llvm/utils/unittest/googletest/LICENSE.TXT
  llvm/utils/unittest/googletest/README.LLVM
  llvm/utils/unittest/googletest/include/gtest/gtest-death-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest-matchers.h
  llvm/utils/unittest/googletest/include/gtest/gtest-message.h
  llvm/utils/unittest/googletest/include/gtest/gtest-param-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest-printers.h
  llvm/utils/unittest/googletest/include/gtest/gtest-spi.h
  llvm/utils/unittest/googletest/include/gtest/gtest-test-part.h
  llvm/utils/unittest/googletest/include/gtest/gtest-typed-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest.h
  llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h
  llvm/utils/unittest/googletest/include/gtest/gtest_prod.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-port.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h
  
llvm/utils/unittest/googletest/include/gtest/internal/gtest-death-test-internal.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-filepath.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-internal.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-param-util.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-port-arch.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-string.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-type-util.h
  llvm/utils/unittest/googletest/src/gtest-all.cc
  llvm/utils/unittest/googletest/src/gtest-death-test.cc
  llvm/utils/unittest/googletest/src/gtest-filepath.cc
  llvm/utils/unittest/googletest/src/gtest-internal-inl.h
  llvm/utils/unittest/googletest/src/gtest-matchers.cc
  llvm/utils/unittest/googletest/src/gtest-port.cc
  llvm/utils/unittest/googletest/src/gtest-printers.cc
  llvm/utils/unittest/googletest/src/gtest-test-part.cc
  llvm/utils/unittest/googletest/src/gtest-typed-test.cc
  llvm/utils/unittest/googletest/src/gtest.cc
  mlir/CMakeLists.txt
  polly/CMakeLists.txt
  third-party/unittest/CMakeLists.txt
  third-party/unittest/UnitTestMain/CMakeLists.txt
  third-party/unittest/UnitTestMain/TestMain.cpp
  third-party/unittest/googlemock/LICENSE.txt
  third-party/unittest/googlemock/README.LLVM
  

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Fangrui Song via Phabricator via lldb-commits]

MaskRay accepted this revision as: MaskRay.
MaskRay added a comment.

I think `if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)` checks for 
standalone builds is not necessary. The check in `llvm/CMakeLists.txt` suffices.
It's unlikely the users will use different cmake versions to configure llvm and 
a subproject like clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Mark de Wever via Phabricator via lldb-commits]

Mordante added a comment.

In D137724#3917644 , @MaskRay wrote:

> I think `if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)` checks for 
> standalone builds is not necessary. The check in `llvm/CMakeLists.txt` 
> suffices.
> It's unlikely the users will use different cmake versions to configure llvm 
> and a subproject like clang.

I can remove the others, but we need to keep the one in 
`runtimes/CMakeLists.txt` too. The runtimes (libc++, libc++abi, and libunwind) 
can be build without building or configuring LLVM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Mark de Wever via Phabricator via lldb-commits]

Mordante added a comment.

In D137724#3917616 , @thieta wrote:

> I think this is fine as we have discussed before. But I really dislike the 
> code duplication for the check. We could put it in a include() I guess - but 
> maybe it's not worth it.

I wanted to make sure it shows up in different build scenarios. The code is 
temporary, once we switch to CMake 3.20.0 we can just change the 
`cmake_minimum_required` and remove these verbose blocks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Tobias Hieta via Phabricator via lldb-commits]

thieta accepted this revision as: thieta.
thieta added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, JDevlieghere.

I think this is fine as we have discussed before. But I really dislike the code 
duplication for the check. We could put it in a include() I guess - but maybe 
it's not worth it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137724/new/

https://reviews.llvm.org/D137724

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137724: [CMake] Warn when the version is older than 3.20.0.

[Mark de Wever via Phabricator via lldb-commits]

Mordante created this revision.
Herald added a reviewer: bollu.
Herald added subscribers: Moerafaat, zero9178, Enna1, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini.
Herald added projects: Flang, All.
Mordante edited the summary of this revision.
Mordante added reviewers: libc++ vendors, clang-vendors, tstellar, mehdi_amini, 
MaskRay, ChuanqiXu, to268, kparzysz, thieta, tschuett, mgorny, stellaraccident, 
mizvekov, ldionne.
Herald added subscribers: StephenFan, jdoerfert.
Mordante published this revision for review.
Herald added subscribers: llvm-commits, libcxx-commits, openmp-commits, 
lldb-commits, Sanitizers, cfe-commits, stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

This is a preparation to require CMake 3.20.0 after LLVM 16 has been
released.

This change has been discussed on discourse
https://discourse.llvm.org/t/rfc-upgrading-llvms-minimum-required-cmake-version/66193


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137724

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/crt/CMakeLists.txt
  flang/CMakeLists.txt
  flang/runtime/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/tools/debugserver/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/docs/ReleaseNotes.rst
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt
  runtimes/CMakeLists.txt

Index: runtimes/CMakeLists.txt
===
--- runtimes/CMakeLists.txt
+++ runtimes/CMakeLists.txt
@@ -1,5 +1,12 @@
 # This file handles building LLVM runtime sub-projects.
 cmake_minimum_required(VERSION 3.13.4)
+if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
+  message(WARNING
+"Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
+"minimum version of CMake required to build LLVM will become 3.20.0, and "
+"using an older CMake will become an error. Please upgrade your CMake to "
+"at least 3.20.0 now to avoid issues in the future!")
+endif()
 project(Runtimes C CXX ASM)
 
 # Add path for custom and the LLVM build's modules to the CMake module path.
Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -2,6 +2,13 @@
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
+  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
+message(WARNING
+  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
+  "minimum version of CMake required to build LLVM will become 3.20.0, and "
+  "using an older CMake will become an error. Please upgrade your CMake to "
+  "at least 3.20.0 now to avoid issues in the future!")
+  endif()
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -12,6 +12,13 @@
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
   set(OPENMP_STANDALONE_BUILD TRUE)
   project(openmp C CXX)
+  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
+message(WARNING
+  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
+  "minimum version of CMake required to build LLVM will become 3.20.0, and "
+  "using an older CMake will become an error. Please upgrade your CMake to "
+  "at least 3.20.0 now to avoid issues in the future!")
+  endif()
 endif()
 
 # Must go below project(..)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -11,6 +11,13 @@
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
+  if ("${CMAKE_VERSION}" VERSION_LESS "3.20.0")
+message(WARNING
+  "Your CMake version is ${CMAKE_VERSION}. Starting with LLVM 17.0.0, the "
+  "minimum version of CMake required to build LLVM will become 3.20.0, and "
+  "using an older CMake will become an error. Please upgrade your CMake to "
+  "at least 3.20.0 now to avoid issues in the future!")
+  endif()
 endif()
 
 # Must go below project(..)
Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -64,6 +64,17 @@
 * Apple Clang >= 10.0
 * Visual Studio 2019 >= 16.7
 
+With LLVM 16.x we will raise the version requirement of CMake used to build
+LLVM. The new requirements are as follows:
+
+* CMake >= 3.20.0
+
+In LLVM 16.x 

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Sam James via Phabricator via lldb-commits]

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 rG32a2af44e1e8: [CMake] Fix -Wstrict-prototypes (authored by 
thesamesam).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

Files:
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/builtins/CMakeLists.txt
  libcxx/cmake/config-ix.cmake
  libcxxabi/cmake/config-ix.cmake
  libunwind/cmake/config-ix.cmake
  lldb/tools/debugserver/source/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindFFI.cmake
  llvm/cmake/modules/FindTerminfo.cmake
  llvm/cmake/modules/FindZ3.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  openmp/runtime/cmake/config-ix.cmake
  polly/lib/External/CMakeLists.txt

Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -64,7 +64,7 @@
 check_c_source_compiles("
 ${_includes}
 ${_type} typeVar;
-int main() {
+int main(void) {
 return 0;
 }
 " ${_variable})
@@ -73,7 +73,7 @@
 
   check_c_source_compiles("
   int func(void) __attribute__((__warn_unused_result__));
-  int main() { return 0; }
+  int main(void) { return 0; }
   " HAS_ATTRIBUTE_WARN_UNUSED_RESULT)
   set(GCC_WARN_UNUSED_RESULT)
   if (HAS_ATTRIBUTE_WARN_UNUSED_RESULT)
@@ -82,22 +82,22 @@
 
   check_c_source_compiles("
   __attribute__ ((unused)) static void foo(void);
-  int main() { return 0; }
+  int main(void) { return 0; }
   " HAVE___ATTRIBUTE__)
 
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)ffs(0); return 0; }
+  int main(void) { (void)ffs(0); return 0; }
   " HAVE_DECL_FFS)
 
   check_c_source_compiles_numeric("
-  int main() { (void)__builtin_ffs(0); return 0; }
+  int main(void) { (void)__builtin_ffs(0); return 0; }
   " HAVE_DECL___BUILTIN_FFS)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_BitScanForward(NULL, 0); return 0; }
+  int main(void) { (void)_BitScanForward(NULL, 0); return 0; }
   " HAVE_DECL__BITSCANFORWARD)
 
   if (NOT HAVE_DECL_FFS AND
@@ -109,12 +109,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)strcasecmp(\"\", \"\"); return 0; }
+  int main(void) { (void)strcasecmp(\"\", \"\"); return 0; }
   " HAVE_DECL_STRCASECMP)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_stricmp(\"\", \"\"); return 0; }
+  int main(void) { (void)_stricmp(\"\", \"\"); return 0; }
   " HAVE_DECL__STRICMP)
 
   if (NOT HAVE_DECL_STRCASECMP AND NOT HAVE_DECL__STRICMP)
@@ -124,12 +124,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)strncasecmp(\"\", \"\", 0); return 0; }
+  int main(void) { (void)strncasecmp(\"\", \"\", 0); return 0; }
   " HAVE_DECL_STRNCASECMP)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_strnicmp(\"\", \"\", 0); return 0; }
+  int main(void) { (void)_strnicmp(\"\", \"\", 0); return 0; }
   " HAVE_DECL__STRNICMP)
 
   if (NOT HAVE_DECL_STRNCASECMP AND NOT HAVE_DECL__STRNICMP)
@@ -139,12 +139,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { snprintf((void*)0, 0, \" \"); return 0; }
+  int main(void) { snprintf((void*)0, 0, \" \"); return 0; }
   " HAVE_DECL_SNPRINTF)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { _snprintf((void*)0, 0, \" \"); return 0; }
+  int main(void) { _snprintf((void*)0, 0, \" \"); return 0; }
   " HAVE_DECL__SNPRINTF)
 
   if (NOT HAVE_DECL_SNPRINTF AND NOT HAVE_DECL__SNPRINTF)
Index: openmp/runtime/cmake/config-ix.cmake
===
--- openmp/runtime/cmake/config-ix.cmake
+++ openmp/runtime/cmake/config-ix.cmake
@@ -27,7 +27,7 @@
 void func2() { printf(\"World\"); }
 __asm__(\".symver func1, func@VER1\");
 __asm__(\".symver func2, func@VER2\");
-int main() {
+int main(void) {
   func1();
   func2();
   return 0;
Index: llvm/cmake/modules/HandleLLVMOptions.cmake
===
--- llvm/cmake/modules/HandleLLVMOptions.cmake
+++ llvm/cmake/modules/HandleLLVMOptions.cmake
@@ -779,7 +779,7 @@
   # line is also a // comment.
   set(OLD_CMAKE_REQUIRED_FLAGS ${CMAKE_REQUIRED_FLAGS})
   set(CMAKE_REQUIRED_FLAGS "${CMAKE_REQUIRED_FLAGS} -Werror -Wcomment")
-  CHECK_C_SOURCE_COMPILES("// \\n//\\nint main() {return 0;}"
+  CHECK_C_SOURCE_COMPILES("// \\n//\\nint main(void) {return 0;}"
   C_WCOMMENT_ALLOWS_LINE_WRAP)
   set(CMAKE_REQUIRED_FLAGS ${OLD_CMAKE_REQUIRED_FLAGS})
   if (NOT C_WCOMMENT_ALLOWS_LINE_WRAP)
Index: 

[Lldb-commits] [PATCH] D133890: [CMake] Do these replacements to make use of D132608

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 473684.
Ericson2314 added a comment.
Herald added a subscriber: Moerafaat.

Rebase, rename variables


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133890/new/

https://reviews.llvm.org/D133890

Files:
  bolt/tools/driver/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  clang/tools/scan-view/CMakeLists.txt
  flang/CMakeLists.txt
  flang/cmake/modules/CMakeLists.txt
  flang/tools/f18/CMakeLists.txt
  libc/test/utils/tools/WrapperGen/CMakeLists.txt
  libcxxabi/CMakeLists.txt
  lld/cmake/modules/CMakeLists.txt
  lldb/bindings/python/CMakeLists.txt
  lldb/cmake/modules/LLDBConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake
  mlir/cmake/modules/CMakeLists.txt
  mlir/examples/standalone/CMakeLists.txt
  mlir/test/CMakeLists.txt
  mlir/utils/mbr/CMakeLists.txt
  openmp/libomptarget/plugins/remote/CMakeLists.txt

Index: openmp/libomptarget/plugins/remote/CMakeLists.txt
===
--- openmp/libomptarget/plugins/remote/CMakeLists.txt
+++ openmp/libomptarget/plugins/remote/CMakeLists.txt
@@ -26,7 +26,7 @@
 
 if (Protobuf_FOUND AND gRPC_FOUND AND PROTOC AND GRPC_CPP_PLUGIN)
   libomptarget_say("Building remote offloading plugin.")
-  set(directory "${CMAKE_BINARY_DIR}/include/openmp/libomptarget/plugins/remote/")
+  set(directory "${LLVMPROJ_BINARY_INCLUDEDIR}/openmp/libomptarget/plugins/remote/")
   file(MAKE_DIRECTORY ${directory})
   execute_process(COMMAND ${CMAKE_COMMAND} -E make_directory ${directory})
   execute_process(
Index: mlir/utils/mbr/CMakeLists.txt
===
--- mlir/utils/mbr/CMakeLists.txt
+++ mlir/utils/mbr/CMakeLists.txt
@@ -1 +1 @@
-configure_file(mlir-mbr.in ${CMAKE_BINARY_DIR}/bin/mlir-mbr @ONLY)
+configure_file(mlir-mbr.in ${LLVMPROJ_BINARY_BINDIR}/mlir-mbr @ONLY)
Index: mlir/test/CMakeLists.txt
===
--- mlir/test/CMakeLists.txt
+++ mlir/test/CMakeLists.txt
@@ -8,7 +8,7 @@
 # Provide the MLIR CMake module dir so that the out of tree Standalone
 # dialect and can add it to the module path.
 set(MLIR_CMAKE_DIR
-  "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/mlir")
+  "${LLVMPROJ_BINARY_LIBDIR}/cmake/mlir")
 
 # Passed to lit.site.cfg.py.in to set up the path where to find libraries.
 set(MLIR_LIB_DIR ${CMAKE_LIBRARY_OUTPUT_DIRECTORY})
Index: mlir/examples/standalone/CMakeLists.txt
===
--- mlir/examples/standalone/CMakeLists.txt
+++ mlir/examples/standalone/CMakeLists.txt
@@ -10,8 +10,8 @@
 message(STATUS "Using MLIRConfig.cmake in: ${MLIR_DIR}")
 message(STATUS "Using LLVMConfig.cmake in: ${LLVM_DIR}")
 
-set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/bin)
-set(LLVM_LIBRARY_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/lib)
+set(LLVM_RUNTIME_OUTPUT_INTDIR ${LLVMPROJ_BINARY_BINDIR})
+set(LLVM_LIBRARY_OUTPUT_INTDIR ${LLVMPROJ_BINARY_LIBDIR})
 set(MLIR_BINARY_DIR ${CMAKE_BINARY_DIR})
 
 list(APPEND CMAKE_MODULE_PATH "${MLIR_CMAKE_DIR}")
Index: mlir/cmake/modules/CMakeLists.txt
===
--- mlir/cmake/modules/CMakeLists.txt
+++ mlir/cmake/modules/CMakeLists.txt
@@ -9,7 +9,7 @@
 set(MLIR_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/mlir" CACHE STRING
   "Path for CMake subdirectory for Polly (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/polly')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(mlir_cmake_builddir "${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/mlir")
+set(mlir_cmake_builddir "${LLVMPROJ_BINARY_LIBDIR}/cmake/mlir")
 
 # Keep this in sync with llvm/cmake/CMakeLists.txt!
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
Index: lldb/cmake/modules/LLDBStandalone.cmake
===
--- lldb/cmake/modules/LLDBStandalone.cmake
+++ lldb/cmake/modules/LLDBStandalone.cmake
@@ -79,7 +79,7 @@
 
 set(CMAKE_INCLUDE_CURRENT_DIR ON)
 include_directories(
-  "${CMAKE_BINARY_DIR}/include"
+  "${LLVMPROJ_BINARY_INCLUDEDIR}"
   "${LLVM_INCLUDE_DIRS}"
   "${CLANG_INCLUDE_DIRS}")
 
@@ -109,6 +109,6 @@
   set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
 endif()
 
-set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
-set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
-set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX})
+set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${LLVMPROJ_BINARY_BINDIR})
+set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${LLVMPROJ_BINARY_LIBDIR})
+set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${LLVMPROJ_BINARY_LIBDIR})
Index: lldb/cmake/modules/LLDBConfig.cmake

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Tom leet via Phabricator via lldb-commits]

Rageking8 added a comment.

In D137338#3912135 , @aaron.ballman 
wrote:

> In D137338#3907281 , @Rageking8 
> wrote:
>
>> I am ok with you guys taking the parts of this revision that you reviewed 
>> and directly commiting them to the repo, just like how the person above did 
>> it. This is to break the revision up to smaller parts.
>
> What name and email address would you like us to use for patch attribution 
> when landing these bits?

Rageking8
tomlee...@gmail.com


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added a comment.

I have done the deduping @phosek requested, and changed the variable names from 
`CMAKE_*` to `LLVMPROJ_*` which hopefully satisfies everyone's criteria. Happy 
with other non `LLVM_` options too.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 473679.
Ericson2314 added a comment.
Herald added a subscriber: Moerafaat.

Dedup code, rename variables


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

Files:
  bolt/CMakeLists.txt
  clang/CMakeLists.txt
  clang/tools/scan-build/CMakeLists.txt
  cmake/Modules/LLVMGNUDirs.cmake
  cmake/Modules/LLVMSetIntDirs.cmake
  compiler-rt/CMakeLists.txt
  compiler-rt/cmake/base-config-ix.cmake
  flang/CMakeLists.txt
  libc/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lldb/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  mlir/CMakeLists.txt
  openmp/CMakeLists.txt
  polly/CMakeLists.txt

Index: polly/CMakeLists.txt
===
--- polly/CMakeLists.txt
+++ polly/CMakeLists.txt
@@ -1,12 +1,25 @@
 # Check if this is a in tree build.
+
+set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 if (NOT DEFINED LLVM_MAIN_SRC_DIR)
   project(Polly)
   cmake_minimum_required(VERSION 3.13.4)
   set(POLLY_STANDALONE_BUILD TRUE)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if(POLLY_STANDALONE_BUILD)
   # Where is LLVM installed?
@@ -48,18 +61,10 @@
   set(POLLY_GTEST_AVAIL 1)
 endif ()
 
-set(POLLY_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
-set(POLLY_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
-
-if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
-  set(LLVM_COMMON_CMAKE_UTILS ${POLLY_SOURCE_DIR}/../cmake)
-endif()
-
 # Make sure that our source directory is on the current cmake module path so that
 # we can include cmake files from this directory.
 list(INSERT CMAKE_MODULE_PATH 0
   "${POLLY_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
   )
 
 include("polly_macros")
Index: openmp/CMakeLists.txt
===
--- openmp/CMakeLists.txt
+++ openmp/CMakeLists.txt
@@ -1,12 +1,5 @@
 cmake_minimum_required(VERSION 3.13.4)
 
-set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
-
-# Add path for custom modules
-list(INSERT CMAKE_MODULE_PATH 0
-  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
 
 # llvm/runtimes/ will set OPENMP_STANDALONE_BUILD.
 if (OPENMP_STANDALONE_BUILD OR "${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
@@ -14,8 +7,18 @@
   project(openmp C CXX)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if (OPENMP_STANDALONE_BUILD)
   # CMAKE_BUILD_TYPE was not set, default to Release.
@@ -51,6 +54,11 @@
   endif()
 endif()
 
+# Add path for custom modules
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake"
+  )
+
 # Check and set up common compiler flags.
 include(config-ix)
 include(HandleOpenMPOptions)
Index: mlir/CMakeLists.txt
===
--- mlir/CMakeLists.txt
+++ mlir/CMakeLists.txt
@@ -7,14 +7,26 @@
 include(${LLVM_COMMON_CMAKE_UTILS}/Modules/CMakePolicy.cmake
   NO_POLICY_SCOPE)
 
+set(MLIR_SOURCE_DIR ${CMAKE_CURRENT_SOURCE_DIR})
+set(MLIR_BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR})
+
 # Check if MLIR is built as a standalone project.
 if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
   project(mlir)
   set(MLIR_STANDALONE_BUILD TRUE)
 endif()
 
-# Must go below project(..)
-include(GNUInstallDirs)
+if(NOT DEFINED LLVM_COMMON_CMAKE_UTILS)
+  set(LLVM_COMMON_CMAKE_UTILS ${CMAKE_CURRENT_SOURCE_DIR}/../cmake)
+endif()
+
+list(INSERT CMAKE_MODULE_PATH 0
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
+
+# Must go before the first `include(GNUInstallDirs)`.
+# Must go after project(..)
+include(LLVMGNUDirs)
 
 if(MLIR_STANDALONE_BUILD)
   find_package(LLVM CONFIG REQUIRED)
@@ -43,19 +55,16 @@
 "Path for binary subdirectory (defaults to '${CMAKE_INSTALL_BINDIR}')")
 mark_as_advanced(MLIR_TOOLS_INSTALL_DIR)
 
-set(MLIR_MAIN_SRC_DIR ${CMAKE_CURRENT_SOURCE_DIR}  )
+set(MLIR_MAIN_SRC_DIR ${MLIR_SOURCE_DIR}   )
 set(MLIR_MAIN_INCLUDE_DIR ${MLIR_MAIN_SRC_DIR}/include )
 
-set(MLIR_SOURCE_DIR  ${CMAKE_CURRENT_SOURCE_DIR})
-set(MLIR_BINARY_DIR  

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

In D137503#3910651 , @aaron.ballman 
wrote:

> LGTM, thank you for this! If we do a 15.0.5, I think these changes should 
> probably go into there as well (WDYT?)

I support this. This will make llvm-project 15.0.5 buildable by more future 
compilers.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

In D137338#3907281 , @Rageking8 wrote:

> I am ok with you guys taking the parts of this revision that you reviewed and 
> directly commiting them to the repo, just like how the person above did it. 
> This is to break the revision up to smaller parts.

What name and email address would you like us to use for patch attribution when 
landing these bits?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you for this! If we do a 15.0.5, I think these changes should 
probably go into there as well (WDYT?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Petr Hosek via Phabricator via lldb-commits]

phosek accepted this revision.
phosek added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Sam James via Phabricator via lldb-commits]

thesamesam created this revision.
thesamesam added reviewers: mgorny, aaron.ballman.
Herald added a reviewer: bollu.
Herald added subscribers: libcxx-commits, Enna1, pengfei.
Herald added projects: libunwind, All.
Herald added a reviewer: libunwind.
thesamesam requested review of this revision.
Herald added projects: Sanitizers, LLDB, libc++, OpenMP, libc++abi, LLVM.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

Fixes warnings (or errors, if someone injects -Werror in their build system,
which happens in fact with some folks vendoring LLVM too) with Clang 16:

  
+/var/tmp/portage.notmp/portage/sys-devel/llvm-15.0.4/work/llvm_build-abi_x86_64.amd64/CMakeFiles/CMakeTmp/src.c:3:9:
 warning: a function declaration without a prototype
  is deprecated in all versions of C [-Wstrict-prototypes]
  
-/var/tmp/portage.notmp/portage/sys-devel/llvm-14.0.4/work/llvm_build-abi_x86_64.amd64/CMakeFiles/CMakeTmp/src.c:3:9:
 error: a function declaration without a prototype is
  deprecated in all versions of C [-Werror,-Wstrict-prototypes]
   int main() {return 0;}
   ^
void


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137503

Files:
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/builtins/CMakeLists.txt
  libcxx/cmake/config-ix.cmake
  libcxxabi/cmake/config-ix.cmake
  libunwind/cmake/config-ix.cmake
  lldb/tools/debugserver/source/CMakeLists.txt
  llvm/cmake/config-ix.cmake
  llvm/cmake/modules/FindFFI.cmake
  llvm/cmake/modules/FindTerminfo.cmake
  llvm/cmake/modules/FindZ3.cmake
  llvm/cmake/modules/HandleLLVMOptions.cmake
  openmp/runtime/cmake/config-ix.cmake
  polly/lib/External/CMakeLists.txt

Index: polly/lib/External/CMakeLists.txt
===
--- polly/lib/External/CMakeLists.txt
+++ polly/lib/External/CMakeLists.txt
@@ -64,7 +64,7 @@
 check_c_source_compiles("
 ${_includes}
 ${_type} typeVar;
-int main() {
+int main(void) {
 return 0;
 }
 " ${_variable})
@@ -73,7 +73,7 @@
 
   check_c_source_compiles("
   int func(void) __attribute__((__warn_unused_result__));
-  int main() { return 0; }
+  int main(void) { return 0; }
   " HAS_ATTRIBUTE_WARN_UNUSED_RESULT)
   set(GCC_WARN_UNUSED_RESULT)
   if (HAS_ATTRIBUTE_WARN_UNUSED_RESULT)
@@ -82,22 +82,22 @@
 
   check_c_source_compiles("
   __attribute__ ((unused)) static void foo(void);
-  int main() { return 0; }
+  int main(void) { return 0; }
   " HAVE___ATTRIBUTE__)
 
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)ffs(0); return 0; }
+  int main(void) { (void)ffs(0); return 0; }
   " HAVE_DECL_FFS)
 
   check_c_source_compiles_numeric("
-  int main() { (void)__builtin_ffs(0); return 0; }
+  int main(void) { (void)__builtin_ffs(0); return 0; }
   " HAVE_DECL___BUILTIN_FFS)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_BitScanForward(NULL, 0); return 0; }
+  int main(void) { (void)_BitScanForward(NULL, 0); return 0; }
   " HAVE_DECL__BITSCANFORWARD)
 
   if (NOT HAVE_DECL_FFS AND
@@ -109,12 +109,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)strcasecmp(\"\", \"\"); return 0; }
+  int main(void) { (void)strcasecmp(\"\", \"\"); return 0; }
   " HAVE_DECL_STRCASECMP)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_stricmp(\"\", \"\"); return 0; }
+  int main(void) { (void)_stricmp(\"\", \"\"); return 0; }
   " HAVE_DECL__STRICMP)
 
   if (NOT HAVE_DECL_STRCASECMP AND NOT HAVE_DECL__STRICMP)
@@ -124,12 +124,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)strncasecmp(\"\", \"\", 0); return 0; }
+  int main(void) { (void)strncasecmp(\"\", \"\", 0); return 0; }
   " HAVE_DECL_STRNCASECMP)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { (void)_strnicmp(\"\", \"\", 0); return 0; }
+  int main(void) { (void)_strnicmp(\"\", \"\", 0); return 0; }
   " HAVE_DECL__STRNICMP)
 
   if (NOT HAVE_DECL_STRNCASECMP AND NOT HAVE_DECL__STRNICMP)
@@ -139,12 +139,12 @@
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { snprintf((void*)0, 0, \" \"); return 0; }
+  int main(void) { snprintf((void*)0, 0, \" \"); return 0; }
   " HAVE_DECL_SNPRINTF)
 
   check_c_source_compiles_numeric("
   #include 
-  int main() { _snprintf((void*)0, 0, \" \"); return 0; }
+  int main(void) { _snprintf((void*)0, 0, \" \"); return 0; }
   " HAVE_DECL__SNPRINTF)
 
   if (NOT HAVE_DECL_SNPRINTF AND NOT HAVE_DECL__SNPRINTF)
Index: openmp/runtime/cmake/config-ix.cmake
===
--- openmp/runtime/cmake/config-ix.cmake
+++ openmp/runtime/cmake/config-ix.cmake
@@ -27,7 +27,7 @@
 void func2() { printf(\"World\"); }
 __asm__(\".symver func1, func@VER1\");
 __asm__(\".symver 

[Lldb-commits] [PATCH] D137503: [CMake] Fix -Wstrict-prototypes

[Sam James via Phabricator via lldb-commits]

thesamesam added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: sstefan1, JDevlieghere.

Note that we of course don't have to bother doing this for C++ sources and test 
programs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137503/new/

https://reviews.llvm.org/D137503

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added inline comments.



Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {

mgorny wrote:
> Well, one thing I immediately notice is that technically 
> `CMAKE_INSTALL_LIBDIR` can be an absolute path, while in many locations you 
> are assuming it will always be relative. Not saying that's a blocker but I 
> think you should add checks for that into `CMakeLists.txt`.
Yes I was working on this, and I intend to use `CMAKE_INSTALL_LIBDIR` with an 
absolute path.

OK if this isn't supported initially but we should ovoid regressing where 
possible.



Comment at: clang/include/clang/Config/config.h.cmake:39
 /* Multilib basename for libdir. */
-#define CLANG_INSTALL_LIBDIR_BASENAME "${CLANG_INSTALL_LIBDIR_BASENAME}"
 

I had called it `_BASENAME` in anticipation that it would be the basename of 
`CMAKE_INSTALL_LIBDIR` eventually when `CMAKE_INSTALL_LIBDIR` is not a relative 
path.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Tom leet via Phabricator via lldb-commits]

Rageking8 added a comment.

I am ok with you guys taking the parts of this revision that you reviewed and 
directly commiting them to the repo, just like how the person above did it. 
This is to break the revision up to smaller parts.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131919: Move googletest to the third-party directory

[Paul Robinson via Phabricator via lldb-commits]

probinson accepted this revision.
probinson added a comment.

Yes, LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131919/new/

https://reviews.llvm.org/D131919

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Marek Kurdej via Phabricator via lldb-commits]

curdeius added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

aaron.ballman wrote:
> I think this should read:
> ```
>  // Handle C++ base classes. Non-virtual bases can treated as a kind of
> ```
A mistake: can *be* treated.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

curdeius wrote:
> aaron.ballman wrote:
> > I think this should read:
> > ```
> >  // Handle C++ base classes. Non-virtual bases can treated as a kind of
> > ```
> A mistake: can *be* treated.
Oh gosh, good catch! 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131919: Move googletest to the third-party directory

[Tom Stellard via Phabricator via lldb-commits]

tstellar added a comment.
Herald added a subscriber: Moerafaat.

@probinson Does this latest update look better?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131919/new/

https://reviews.llvm.org/D131919

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Aaron Ballman via Phabricator via lldb-commits]

aaron.ballman added a comment.

The clang parts generally LG, but I spotted a few things. Thank you for this 
cleanup effort!




Comment at: clang/include/clang/ASTMatchers/ASTMatchersInternal.h:467
 
-  /// Return a matcher that that points to the same implementation, but sets 
the
+  /// Return a matcher that points to the same implementation, but sets the
   ///   traversal kind.

This is a good change, but you also need to run 
`clang/docs/tools/dump_ast_matchers.py` to regenerate the public-facing 
documentation that is generated from this file.



Comment at: clang/lib/CodeGen/CodeGenTBAA.cpp:341
 if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
-  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // Handle C++ base classes. Non-virtual bases can treated a kind of
   // field. Virtual bases are more complex and omitted, but avoid an

I think this should read:
```
 // Handle C++ base classes. Non-virtual bases can treated as a kind of
```



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:47
   // complete architecture list, nor a reasonable subset. The problem is that
-  // historically the driver driver accepts this and also ties its -march=
   // handling to the architecture name, so we need to be careful before 
removing

This is duplicated just often enough and in just enough contexts where I think 
"driver driver" means "the driver that drives another driver", but it'd be nice 
if someone else could confirm or deny that.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:87
 
-  // If using a driver driver, force the arch.
   if (getToolChain().getTriple().isOSDarwin()) {

Same for this one as above.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Jez Ng via Phabricator via lldb-commits]

int3 added a comment.

lld/MachO changes lgtm




Comment at: lld/MachO/Driver.cpp:1386
 // ld64 does something really weird. It attempts to realign the value to 
the
-// page size, but assumes the the page size is 4K. This doesn't work with
+// page size, but assumes the page size is 4K. This doesn't work with
 // most of Apple's ARM64 devices, which use a page size of 16K. This means

"assumes that the page size" sounds more natural and is probably what was 
intended



Comment at: lld/MachO/UnwindInfoSection.cpp:576-577
 // entries by using the regular format. This can happen when there
-// are many unique encodings, and we we saturated the local
+// are many unique encodings, and we saturated the local
 // encoding table early.
 if (i < cuIndices.size() &&

I think we can reflow this paragraph to max the line length


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[Shilei Tian via Phabricator via lldb-commits]

tianshilei1992 added a comment.

Can you verify if projects enabled by `LLVM_ENABLE_RUNTIMES` (not 
`LLVM_ENABLE_PROJECTS`) will still be installed properly, aka in LLVM's lib 
directory?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Amir Ayupov via Phabricator via lldb-commits]

Amir accepted this revision.
Amir added a comment.

BOLT changes LGTM


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[Michał Górny via Phabricator via lldb-commits]

mgorny added inline comments.



Comment at: bolt/lib/RuntimeLibs/RuntimeLibrary.cpp:32
   SmallString<128> LibPath = llvm::sys::path::parent_path(Dir);
-  llvm::sys::path::append(LibPath, "lib" LLVM_LIBDIR_SUFFIX);
+  llvm::sys::path::append(LibPath, CMAKE_INSTALL_LIBDIR);
   if (!llvm::sys::fs::exists(LibPath)) {

Well, one thing I immediately notice is that technically `CMAKE_INSTALL_LIBDIR` 
can be an absolute path, while in many locations you are assuming it will 
always be relative. Not saying that's a blocker but I think you should add 
checks for that into `CMakeLists.txt`.



Comment at: clang/tools/libclang/CMakeLists.txt:234
   DESTINATION
-"lib${LLVM_LIBDIR_SUFFIX}/python${PythonVersion}/site-packages")
+  "${CMAKE_INSTALL_LIBDIR}/python${PythonVersion}/site-packages")
 endforeach()

You seem to be changing indentation here, and below.

On unrelated note, hardcoding site-packages path sounds like a bad idea but 
that's a problem for another patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Siva Chandra via Phabricator via lldb-commits]

sivachandra accepted this revision.
sivachandra added a comment.

Changes in the libc directory LGTM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[David Spickett via Phabricator via lldb-commits]

DavidSpickett added a comment.

Looked at the lldb changes, some comments for you. If you want to get a "looks 
good" for those please submit a separate review with only the lldb parts and 
I'll review that instead.

As others said, appreciate the effort but the review process doesn't scale well 
to so many changes in one patch.




Comment at: lldb/include/lldb/Target/Process.h:1204
   /// platform that might itself be running natively, but have different
-  /// heuristics for figuring out which OS is is emulating.
+  /// heuristics for figuring out which OS is emulating.
   ///

This should be "which OS it is emulating".



Comment at: lldb/source/Symbol/LineTable.cpp:92
 // after the prologue.
-// Instead of it it is issuing a line table entry for the first instruction
+// Instead of it is issuing a line table entry for the first instruction
 // of the prologue and one for the first instruction after the prologue. If

This should be "Instead it is issuing".



Comment at: lldb/source/Target/RegisterContextUnwind.cpp:701
   // can have arbitrary number of frames with the same CFA, but more then 2 is
-  // very very unlikely)
 

This one is intended. It's not very scientific but hey, it's getting the point 
across.



Comment at: lldb/tools/lldb-vscode/JSONUtils.cpp:1045
   auto type_cstr = type_obj.GetDisplayTypeName();
-  // If we have a type with many many children, we would like to be able to
   // give a hint to the IDE that the type has indexed children so that the

This one is intended. Given the context of batching work, it's making the point 
for very large amounts of children.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Shilei Tian via Phabricator via lldb-commits]

tianshilei1992 added a comment.

Changes to `openmp` look good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Louis Dionne via Phabricator via lldb-commits]

ldionne accepted this revision.
ldionne added a comment.

Thanks for the fixes. LGTM for `libcxx/`, `libcxxabi/` and `libunwind/`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Michael Kruse via Phabricator via lldb-commits]

Meinersbur added a comment.

Changes in `/polly/` look good to me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Aart Bik via Phabricator via lldb-commits]

aartbik added a comment.

LGTM for sparse changes

(note that you could have made your life a bit easier if you had broken this 
revision up at least over different projects, getting a global "LGTM" from 
somebody may be a bit hard now ;-)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[serge via Phabricator via lldb-commits]

serge-sans-paille updated this revision to Diff 472945.
serge-sans-paille added a comment.

+ Release Note


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

Files:
  bolt/CMakeLists.txt
  bolt/include/bolt/RuntimeLibs/RuntimeLibraryVariables.inc.in
  bolt/lib/RuntimeLibs/RuntimeLibrary.cpp
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/caches/Android-stage2.cmake
  clang/cmake/caches/Android.cmake
  clang/cmake/modules/AddClang.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  cmake/Modules/GNUInstallPackageDir.cmake
  compiler-rt/cmake/Modules/CompilerRTAIXUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/docs/BuildingCompilerRT.rst
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/cmake/modules/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/cmake/modules/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/API/CMakeLists.txt
  lldb/test/CMakeLists.txt
  lldb/tools/intel-features/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/docs/CMake.rst
  llvm/docs/ReleaseNotes.rst
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/utils/gn/secondary/llvm/tools/llvm-config/BUILD.gn
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/cmake/modules/AddMLIRPython.cmake
  mlir/cmake/modules/CMakeLists.txt
  mlir/test/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/README.rst
  openmp/libompd/src/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/plugins-nextgen/CMakeLists.txt
  openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
  openmp/libomptarget/plugins/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/cuda/CMakeLists.txt
  openmp/libomptarget/plugins/remote/src/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/libomptarget/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/Modules/CMakeLists.txt
  openmp/tools/archer/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake

Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@
   if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "LLVMPolly")
 install(TARGETS ${name}
   EXPORT LLVMExports
-  LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-  ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
   endif()
   set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
 endmacro(add_polly_library)
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -7,7 +7,7 @@
 set(POLLY_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/polly" CACHE STRING
   "Path for CMake subdirectory for Polly (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/polly')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(polly_cmake_builddir "${POLLY_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/polly")
+set(polly_cmake_builddir "${POLLY_BINARY_DIR}/${CMAKE_INSTALL_LIBDIR}/cmake/polly")
 
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
@@ -93,7 +93,7 @@
 find_prefix_from_config(POLLY_CONFIG_CODE POLLY_INSTALL_PREFIX "${POLLY_INSTALL_PACKAGE_DIR}")
 extend_path(POLLY_CONFIG_LLVM_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${LLVM_INSTALL_PACKAGE_DIR}")
 extend_path(POLLY_CONFIG_CMAKE_DIR "\${POLLY_INSTALL_PREFIX}" "${POLLY_INSTALL_PACKAGE_DIR}")
-extend_path(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}" "lib${LLVM_LIBDIR_SUFFIX}")
+extend_path(POLLY_CONFIG_LIBRARY_DIRS "\${POLLY_INSTALL_PREFIX}" "${CMAKE_INSTALL_LIBDIR}")
 extend_path(base_includedir "\${POLLY_INSTALL_PREFIX}" "${CMAKE_INSTALL_INCLUDEDIR}")
 if (POLLY_BUNDLED_ISL)
   set(POLLY_CONFIG_INCLUDE_DIRS
Index: openmp/tools/archer/CMakeLists.txt

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Jay Foad via Phabricator via lldb-commits]

foad added a comment.

I committed the lib/Target/AMDGPU parts as 5073ae2a883f 
.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[Alexander Potapenko via Phabricator via lldb-commits]

glider added subscribers: pcc, glider.
glider added inline comments.



Comment at: clang/docs/DataFlowSanitizerDesign.rst:54
 
-  /// Returns whether the given label label contains the label elem.
+  /// Returns whether the given label contains the label elem.
   int dfsan_has_label(dfsan_label label, dfsan_label elem);

I believe the word duplication is on purpose here. I'd rewrite the comment as:

```
Returns whether the given label @label contains the label @elem.
```

@pcc, what do you think?



Comment at: compiler-rt/include/sanitizer/dfsan_interface.h:65
 
-/// Returns whether the given label label contains the label elem.
+/// Returns whether the given label contains the label elem.
 int dfsan_has_label(dfsan_label label, dfsan_label elem);

Same as clang/docs/DataFlowSanitizerDesign.rst above.



Comment at: compiler-rt/lib/asan/asan_allocator.cpp:1161
   // the `pthread_create()` interceptor doesn't wait for the child thread to
-  // start before returning and thus loosing the the only live reference to the
+  // start before returning and thus loosing the only live reference to the
   // heap object on the stack.

Ack.



Comment at: 
compiler-rt/lib/sanitizer_common/tests/sanitizer_stacktrace_test.cpp:211
 
-  // This the the truncation case.
+  // This the truncation case.
   ASSERT_GT(actual_len, sizeof(tinybuf));

Ack.



Comment at: compiler-rt/lib/tsan/rtl-old/tsan_interceptors_posix.cpp:2510
 // Can't use internal_memcpy, because it copies byte-by-byte,
-// and signal handler reads the handler concurrently. It it can read
+// and signal handler reads the handler concurrently. It can read
 // some bytes from old value and some bytes from new value.

Ack.



Comment at: compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp:2529
 // Can't use internal_memcpy, because it copies byte-by-byte,
-// and signal handler reads the handler concurrently. It it can read
+// and signal handler reads the handler concurrently. It can read
 // some bytes from old value and some bytes from new value.

Ack.



Comment at: llvm/lib/Transforms/Instrumentation/DataFlowSanitizer.cpp:540
   Type *getShadowTy(Type *OrigTy);
-  /// Returns the shadow type of of V's type.
+  /// Returns the shadow type of V's type.
   Type *getShadowTy(Value *V);

Ack.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137338: Fix dupe word typos

[James Henderson via Phabricator via lldb-commits]

jhenderson added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: Michael137, sstefan1, JDevlieghere.

This is going to be impossible to cleanly review as-is. Could it be broken into 
lots of smaller chunks?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137338/new/

https://reviews.llvm.org/D137338

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[serge via Phabricator via lldb-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: MaskRay, tstellar, mgorny, sylvestre.ledru.
Herald added subscribers: libc-commits, libcxx-commits, Moerafaat, zero9178, 
Enna1, bzcheeseman, kosarev, ayermolo, sdasgup3, wenzhicui, wrengr, cota, 
StephenFan, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, Joonsoo, kerbowa, liufengdb, aartbik, mgester, arpith-jacob, csigg, 
antiagainst, shauheen, rriddle, mehdi_amini, whisperity, jvesely.
Herald added a reviewer: bollu.
Herald added a reviewer: sscalpone.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added a reviewer: NoQ.
Herald added projects: libunwind, libc-project, Flang, All.
Herald added a reviewer: libunwind.
serge-sans-paille requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: llvm-commits, openmp-commits, lldb-commits, 
Sanitizers, cfe-commits, yota9, sstefan1, stephenneuendorffer, 
nicolasvasilache, jdoerfert.
Herald added projects: clang, Sanitizers, LLDB, libc++, OpenMP, libc++abi, 
MLIR, LLVM.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.

There are a few advantages of using CMAKE_INSTALL_LIBDIR in place of
LLVM_LIBDIR_SUFFIX:

1. it's a standard flag, no need to support an extra cmake configuration
2. it has sane defaults, and picks lib64 instead of lib on some syssyems 
(including Fedora)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D137337

Files:
  bolt/CMakeLists.txt
  bolt/include/bolt/RuntimeLibs/RuntimeLibraryVariables.inc.in
  bolt/lib/RuntimeLibs/RuntimeLibrary.cpp
  bolt/runtime/CMakeLists.txt
  clang/CMakeLists.txt
  clang/cmake/caches/Android-stage2.cmake
  clang/cmake/caches/Android.cmake
  clang/cmake/modules/AddClang.cmake
  clang/cmake/modules/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Headers/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  cmake/Modules/GNUInstallPackageDir.cmake
  compiler-rt/cmake/Modules/CompilerRTAIXUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/docs/BuildingCompilerRT.rst
  flang/CMakeLists.txt
  flang/cmake/modules/AddFlang.cmake
  flang/cmake/modules/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/CMakeLists.txt
  lld/cmake/modules/AddLLD.cmake
  lld/cmake/modules/CMakeLists.txt
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/cmake/modules/LLDBStandalone.cmake
  lldb/source/API/CMakeLists.txt
  lldb/test/CMakeLists.txt
  lldb/tools/intel-features/CMakeLists.txt
  lldb/utils/lldb-dotest/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/docs/CMake.rst
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/utils/gn/secondary/llvm/tools/llvm-config/BUILD.gn
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/cmake/modules/AddMLIRPython.cmake
  mlir/cmake/modules/CMakeLists.txt
  mlir/test/CMakeLists.txt
  openmp/CMakeLists.txt
  openmp/README.rst
  openmp/libompd/src/CMakeLists.txt
  openmp/libomptarget/DeviceRTL/CMakeLists.txt
  openmp/libomptarget/plugins-nextgen/CMakeLists.txt
  openmp/libomptarget/plugins-nextgen/cuda/CMakeLists.txt
  openmp/libomptarget/plugins/CMakeLists.txt
  openmp/libomptarget/plugins/amdgpu/CMakeLists.txt
  openmp/libomptarget/plugins/cuda/CMakeLists.txt
  openmp/libomptarget/plugins/remote/src/CMakeLists.txt
  openmp/libomptarget/plugins/ve/CMakeLists.txt
  openmp/libomptarget/src/CMakeLists.txt
  openmp/runtime/src/CMakeLists.txt
  openmp/tools/Modules/CMakeLists.txt
  openmp/tools/archer/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake

Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@
   if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "LLVMPolly")
 install(TARGETS ${name}
   EXPORT LLVMExports
-  LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-  ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
+  LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+  ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR})
   endif()
   set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
 endmacro(add_polly_library)
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -7,7 +7,7 

[Lldb-commits] [PATCH] D137337: Replace LLVM_LIBDIR_SUFFIX by CMAKE_INSTALL_LIBDIR

[Sylvestre Ledru via Phabricator via lldb-commits]

sylvestre.ledru added a comment.

Maybe add it to the release notes?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137337/new/

https://reviews.llvm.org/D137337

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.
Herald added a subscriber: Moerafaat.

> We held off on this before as `LLVM_LIBDIR_SUFFIX` conflicted with it. Now we 
> return this.
>
> I imagine this is too potentially-breaking to make LLVM 15. That's fine. ...

These sentences are no longer relevant and should be removed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] 3e03873 - [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux

[Zequan Wu via lldb-commits]

That's a known issue for the NativePDB plugin:
https://github.com/llvm/llvm-project/issues/51933. Because currently the
plugin internally looks up function names by full names. Even if you try to
set a breakpoint with a full function name, the plugin itself only receives
the basename.

On Wed, Nov 2, 2022 at 7:03 AM Michael Buch  wrote:

> > For example the type `const char *` also has a
> summary provider, and I'd hope that this feature does not depend on the
> specific way in which that summary is computed.
>
> Yea, it would be nice to have this on Linux. Will try to use a type whose
> format is compatible across platforms
>
> Am Mi., 2. Nov. 2022 um 06:54 Uhr schrieb Pavel Labath :
>
>> On 01/11/2022 06:59, Michael Buch via lldb-commits wrote:
>> >
>> > Author: Michael Buch
>> > Date: 2022-10-31T22:59:16-07:00
>> > New Revision: 3e03873e363b5aa90e4488da63a6de0648d11aba
>> >
>> > URL:
>> https://github.com/llvm/llvm-project/commit/3e03873e363b5aa90e4488da63a6de0648d11aba
>> > DIFF:
>> https://github.com/llvm/llvm-project/commit/3e03873e363b5aa90e4488da63a6de0648d11aba.diff
>> >
>> > LOG: [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux
>> >
>> > * Windows doesn't support setting these breakpoints by basename
>> > * On Linux std::function arguments aren't formatted as such
>>
>> The windows issue seems problematic (Zequan might be interested in
>> that), but the non-pretty-printing of std::function is most likely
>> caused by our lack of a pretty-printer for libstdc++'s (default for
>> linux) std::function. It doesn't seem ideal that a fairly generic test
>> would depend on the existence of a (very complicated) pretty printer.
>>
>> Is there a specific edge case that this particular check was trying to
>> hit? Could it be moved into a separate test case, or ideally replaced by
>> a something simpler? For example the type `const char *` also has a
>> summary provider, and I'd hope that this feature does not depend on the
>> specific way in which that summary is computed.
>>
>> >
>> > Added:
>> >
>> >
>> > Modified:
>> >  lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
>> >
>> > Removed:
>> >
>> >
>> >
>> >
>> 
>> > diff  --git a/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
>> b/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
>> > index ab16c656624d8..d990114f57845 100644
>> > --- a/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
>> > +++ b/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
>> > @@ -1,3 +1,4 @@
>> > +# REQUIRES: system-darwin
>> >   # RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
>> >   # RUN: %lldb -b -s %s %t.out | FileCheck %s
>> >   settings set -f frame-format "frame ${function.name-with-args}\n"
>> >
>> >
>> >
>> > ___
>> > lldb-commits mailing list
>> > lldb-commits@lists.llvm.org
>> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Re: [Lldb-commits] [lldb] 3e03873 - [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux

[Michael Buch via lldb-commits]

> For example the type `const char *` also has a
summary provider, and I'd hope that this feature does not depend on the
specific way in which that summary is computed.

Yea, it would be nice to have this on Linux. Will try to use a type whose
format is compatible across platforms

Am Mi., 2. Nov. 2022 um 06:54 Uhr schrieb Pavel Labath :

> On 01/11/2022 06:59, Michael Buch via lldb-commits wrote:
> >
> > Author: Michael Buch
> > Date: 2022-10-31T22:59:16-07:00
> > New Revision: 3e03873e363b5aa90e4488da63a6de0648d11aba
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/3e03873e363b5aa90e4488da63a6de0648d11aba
> > DIFF:
> https://github.com/llvm/llvm-project/commit/3e03873e363b5aa90e4488da63a6de0648d11aba.diff
> >
> > LOG: [lldb][Test] Fix TestFrameFormatNameWithArgs.test on Windows/Linux
> >
> > * Windows doesn't support setting these breakpoints by basename
> > * On Linux std::function arguments aren't formatted as such
>
> The windows issue seems problematic (Zequan might be interested in
> that), but the non-pretty-printing of std::function is most likely
> caused by our lack of a pretty-printer for libstdc++'s (default for
> linux) std::function. It doesn't seem ideal that a fairly generic test
> would depend on the existence of a (very complicated) pretty printer.
>
> Is there a specific edge case that this particular check was trying to
> hit? Could it be moved into a separate test case, or ideally replaced by
> a something simpler? For example the type `const char *` also has a
> summary provider, and I'd hope that this feature does not depend on the
> specific way in which that summary is computed.
>
> >
> > Added:
> >
> >
> > Modified:
> >  lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
> >
> > Removed:
> >
> >
> >
> >
> 
> > diff  --git a/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
> b/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
> > index ab16c656624d8..d990114f57845 100644
> > --- a/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
> > +++ b/lldb/test/Shell/Settings/TestFrameFormatNameWithArgs.test
> > @@ -1,3 +1,4 @@
> > +# REQUIRES: system-darwin
> >   # RUN: %clangxx_host -g -O0 %S/Inputs/names.cpp -std=c++17 -o %t.out
> >   # RUN: %lldb -b -s %s %t.out | FileCheck %s
> >   settings set -f frame-format "frame ${function.name-with-args}\n"
> >
> >
> >
> > ___
> > lldb-commits mailing list
> > lldb-commits@lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>
>
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D131919: Move googletest to the third-party directory

[Tom Stellard via Phabricator via lldb-commits]

tstellar updated this revision to Diff 468784.
tstellar marked 4 inline comments as done.
tstellar added a comment.
Herald added a subscriber: zero9178.

Rebase and fix some missed path updates.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131919/new/

https://reviews.llvm.org/D131919

Files:
  clang/CMakeLists.txt
  compiler-rt/CMakeLists.txt
  lld/CMakeLists.txt
  lldb/cmake/modules/LLDBStandalone.cmake
  llvm/CMakeLists.txt
  llvm/cmake/modules/HandleLLVMOptions.cmake
  llvm/utils/unittest/CMakeLists.txt
  llvm/utils/unittest/UnitTestMain/CMakeLists.txt
  llvm/utils/unittest/UnitTestMain/TestMain.cpp
  llvm/utils/unittest/googlemock/LICENSE.txt
  llvm/utils/unittest/googlemock/README.LLVM
  llvm/utils/unittest/googlemock/include/gmock/gmock-actions.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-cardinalities.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-function-mocker.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-generated-actions.h
  
llvm/utils/unittest/googlemock/include/gmock/gmock-generated-function-mockers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-generated-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-more-actions.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-more-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-nice-strict.h
  llvm/utils/unittest/googlemock/include/gmock/gmock-spec-builders.h
  llvm/utils/unittest/googlemock/include/gmock/gmock.h
  
llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-generated-actions.h
  llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-matchers.h
  llvm/utils/unittest/googlemock/include/gmock/internal/custom/gmock-port.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-internal-utils.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-port.h
  llvm/utils/unittest/googlemock/include/gmock/internal/gmock-pp.h
  llvm/utils/unittest/googlemock/src/gmock-all.cc
  llvm/utils/unittest/googlemock/src/gmock-cardinalities.cc
  llvm/utils/unittest/googlemock/src/gmock-internal-utils.cc
  llvm/utils/unittest/googlemock/src/gmock-matchers.cc
  llvm/utils/unittest/googlemock/src/gmock-spec-builders.cc
  llvm/utils/unittest/googlemock/src/gmock.cc
  llvm/utils/unittest/googletest/LICENSE.TXT
  llvm/utils/unittest/googletest/README.LLVM
  llvm/utils/unittest/googletest/include/gtest/gtest-death-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest-matchers.h
  llvm/utils/unittest/googletest/include/gtest/gtest-message.h
  llvm/utils/unittest/googletest/include/gtest/gtest-param-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest-printers.h
  llvm/utils/unittest/googletest/include/gtest/gtest-spi.h
  llvm/utils/unittest/googletest/include/gtest/gtest-test-part.h
  llvm/utils/unittest/googletest/include/gtest/gtest-typed-test.h
  llvm/utils/unittest/googletest/include/gtest/gtest.h
  llvm/utils/unittest/googletest/include/gtest/gtest_pred_impl.h
  llvm/utils/unittest/googletest/include/gtest/gtest_prod.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-port.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest-printers.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/gtest.h
  llvm/utils/unittest/googletest/include/gtest/internal/custom/raw-ostream.h
  
llvm/utils/unittest/googletest/include/gtest/internal/gtest-death-test-internal.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-filepath.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-internal.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-param-util.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-port-arch.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-port.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-string.h
  llvm/utils/unittest/googletest/include/gtest/internal/gtest-type-util.h
  llvm/utils/unittest/googletest/src/gtest-all.cc
  llvm/utils/unittest/googletest/src/gtest-death-test.cc
  llvm/utils/unittest/googletest/src/gtest-filepath.cc
  llvm/utils/unittest/googletest/src/gtest-internal-inl.h
  llvm/utils/unittest/googletest/src/gtest-matchers.cc
  llvm/utils/unittest/googletest/src/gtest-port.cc
  llvm/utils/unittest/googletest/src/gtest-printers.cc
  llvm/utils/unittest/googletest/src/gtest-test-part.cc
  llvm/utils/unittest/googletest/src/gtest-typed-test.cc
  llvm/utils/unittest/googletest/src/gtest.cc
  mlir/CMakeLists.txt
  polly/CMakeLists.txt
  third-party/unittest/CMakeLists.txt
  third-party/unittest/UnitTestMain/CMakeLists.txt
  third-party/unittest/UnitTestMain/TestMain.cpp
  third-party/unittest/googlemock/LICENSE.txt
  third-party/unittest/googlemock/README.LLVM
  third-party/unittest/googlemock/include/gmock/gmock-actions.h
  third-party/unittest/googlemock/include/gmock/gmock-cardinalities.h

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

In D128465#3826150 , @mgorny wrote:

> Is anyone working on a solution that would work for people using a zstd 
> install method that's actually supported upstream?

I have created D134990 , it'd be great if 
others could test it on their system.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Michał Górny via Phabricator via lldb-commits]

mgorny added a comment.

Is anyone working on a solution that would work for people using a zstd install 
method that's actually supported upstream?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

In D128465#3800479 , @andrewrk wrote:

>   if(LLVM_ENABLE_ZSTD)
> list(APPEND imported_libs zstd::libzstd_shared)
>   endif()
>
> This hard codes shared linking which breaks the use case of static linking 
> LLVM.

This was addressed by D132870  and D133222 
.

> Also LLVM needs to now include a Findzstd.cmake file or else we get this 
> error:
>
>   CMake Error at cmake/config-ix.cmake:144 (find_package):
> By not providing "Findzstd.cmake" in CMAKE_MODULE_PATH this project has
> asked CMake to find a package configuration file provided by "zstd", but
> CMake did not find one.
>   
> Could not find a package configuration file provided by "zstd" with any of
> the following names:
>   
>   zstdConfig.cmake
>   zstd-config.cmake
>   
> Add the installation prefix of "zstd" to CMAKE_PREFIX_PATH or set
> "zstd_DIR" to a directory containing one of the above files.  If "zstd"
> provides a separate development package or SDK, be sure it has been
> installed.
>   Call Stack (most recent call first):
> CMakeLists.txt:774 (include)
>
> It is impossible to satisfy this dependency when bootstrapping a static build 
> of zig without patching LLVM.

It should be possible to use `CMAKE_FIND_PACKAGE_PREFER_CONFIG=ON` to pick up 
config file that's part of the zstd installation, but I'd be also fine 
including `Findzstd.cmake` in LLVM to avoid needing that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Andrew Kelley via Phabricator via lldb-commits]

andrewrk added a comment.

Compiler infrastructure should not assume the existence of a Linux 
distribution. The portable way is simple and can easily be supported. One 
should be able to install `$prefix/lib/libzstd.a` and `$prefix/include/zstd.h`, 
then have that prefix searched as part of the standard `CMAKE_PREFIX_PATH`.

If LLVM will not carry Findzstd.cmake then it should not use `FIND_PACKAGE` in 
the case of `-DLLVM_ENABLE_ZSTD=FORCE_ON`; it should just throw `-lzstd` on the 
linker line instead of throwing rakes in my path for no reason.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Andrew Kelley via Phabricator via lldb-commits]

andrewrk added a comment.

  if(LLVM_ENABLE_ZSTD)
list(APPEND imported_libs zstd::libzstd_shared)
  endif()

This hard codes shared linking which breaks the use case of static linking LLVM.

Also LLVM needs to now include a Findzstd.cmake file or else we get this error:

  CMake Error at cmake/config-ix.cmake:144 (find_package):
By not providing "Findzstd.cmake" in CMAKE_MODULE_PATH this project has
asked CMake to find a package configuration file provided by "zstd", but
CMake did not find one.
  
Could not find a package configuration file provided by "zstd" with any of
the following names:
  
  zstdConfig.cmake
  zstd-config.cmake
  
Add the installation prefix of "zstd" to CMAKE_PREFIX_PATH or set
"zstd_DIR" to a directory containing one of the above files.  If "zstd"
provides a separate development package or SDK, be sure it has been
installed.
  Call Stack (most recent call first):
CMakeLists.txt:774 (include)

It is impossible to satisfy this dependency when bootstrapping a static build 
of zig without patching LLVM.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added a comment.

zstd provides GNU Makefile, CMake, and Meson. The CMake files are only 
installed when configured with CMake. Debian and Ubuntu lack such files.
The pkg-config file libzstd.pc is probably the most portable file. (I have also 
used it for a binutils-gdb patch.)

I think we can then avoid the

  zstdConfig.cmake
  zstd-config.cmake

issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D128465/new/

https://reviews.llvm.org/D128465

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D133890: [CMake] Do these replacements to make use of D132608

[Saleem Abdulrasool via Phabricator via lldb-commits]

compnerd added a comment.

I don't see anything wrong with this change per se, but I'm conflicted on the 
name of the variable.  These are not standard variables but are encroaching on 
the CMake namespace.  What happens if upstream decides to use these names?  I 
think that we should keep the names in the `LLVM_` namespace.  However, I 
realize that the variable itself is not being touched here and this is a 
mechanical replacement.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133890/new/

https://reviews.llvm.org/D133890

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Petr Hosek via Phabricator via lldb-commits]

phosek added a comment.

I agree with @tianshilei1992, I think we should avoid introducing new `CMAKE_` 
variables to avoid confusion. The same applies to module names, for example I 
don't think we should be introducing `GNUBinaryDirs` which can be easily 
confused for `GNUInstallDirs`.

I would personally go with the `LLVM_` prefix, I agree that this can lead to 
some confusion given that LLVM is both the name of the top-level project as 
well as one of the subprojects, but I think that ship has already sailed we 
already have proliferation of `LLVM_` named variables throughout the codebase.

Regarding `GNUBinaryDirs`, it seems like we always use the following pattern:

  # Must go before the first `include(GNUInstallDirs)`.
  include(LLVMLibdirSuffix)
  
  # Must go below project(..)
  include(GNUInstallDirs)
  include(GNUBinaryDirs)

That introduces a lot of duplication and is potentially error-prone since we 
rely on particular include order. Could we instead introduce a single 
`LLVMInstallDirs` module which would contain the content of  `LLVMLibdirSuffix` 
and `GNUBinaryDirs` and would also include `GNUInstallDirs`, so 
`LLVMInstallDirs` is all you would need to include in individual subprojects?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added a comment.

@tianshilei1992 that is a fair point. I would be open to calling them something 
else.

I just don't want to call them `LLVM_*` because that would be confusing since 
there is (a) LLVM in particular (b) the monorepo / project as a whole, and this 
variable are about *neither* of them, being scoped to the current build.

I mainly named this way to match the`GNUInstallDirs` naming scheme, where 
`CMAKE_INSTALL_PREFIX` goes with` CMAKE_INSTALL_*DIR`; here `CMAKE_BINARY_DIR` 
goes with `CMAKE_BINARY_*DIR`. I admit relative vs absolute paths, in addition 
to `_PREFIX` vs `_DIR`, do not make the analogy exact though.

I also was hoping something like this could be upstreamed into CMake itself, 
but using different names now doesn't prevent upstream from adopting `CMAKE_*` 
were this to become official.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[Shilei Tian via Phabricator via lldb-commits]

tianshilei1992 added a comment.

Is it a good idea to define variables starting with `CMAKE`? That might be 
confusing for later maintenance by other developers because chances are that 
they will think those variables are CMake provided variables, try to look up 
into CMake document to see what they are, and then find out they are not at all.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D132608: [CMake] Clean up CMake binary dir handling

[John Ericson via Phabricator via lldb-commits]

Ericson2314 added inline comments.



Comment at: libcxx/CMakeLists.txt:421
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(default_install_path 
"${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
+else()

ldionne wrote:
> ldionne wrote:
> > Instead, I'd do this:
> > 
> > ```
> > if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
> >   set(_include_target_dir 
> > "${CMAKE_INSTALL_INCLUDEDIR}/${LLVM_DEFAULT_TARGET_TRIPLE}/c++/v1")
> >   set(_install_library_dir 
> > "lib${LLVM_LIBDIR_SUFFIX}/${LLVM_DEFAULT_TARGET_TRIPLE}")
> > else()
> >   set(_include_target_dir "${LIBCXX_INSTALL_INCLUDE_DIR}")
> >   set(_install_library_dir "lib${LLVM_LIBDIR_SUFFIX}")
> > endif()
> > 
> > set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR "${_include_target_dir}" CACHE PATH
> > "Path where target-specific libc++ headers should be installed.")
> > set(LIBCXX_INSTALL_LIBRARY_DIR "${_install_library_dir}" CACHE PATH
> > "Path where built libc++ libraries should be installed.")
> > ```
> > 
> > IMO that's easier on the eye.
> Gentle ping on this.
Ah right, I have this "throw away temporary style as soon as possible" on all 
the runtimes right now. Would you prefer I do instead do "define all the 
defaults, define all the cache vars" for all of them?

I am a little partial to the current style because if it is closer to what I 
would do if cmake has proper expressions. (something like:
```
set(LIBCXX_INSTALL_INCLUDE_TARGET_DIR
  "${
if(...)
  stuff
else()
 stuff
endif()
  }" CACHE PATH
"Path where target-specific libc++ headers should be 
```
). But if that if you are sure your want it consistently the other way I can 
switch it.

A bigger issue might be when the variables depend on each other, e.g. when I 
get the library install dir base name for the library binary dir.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D132608/new/

https://reviews.llvm.org/D132608

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[John Ericson via Phabricator via lldb-commits]

Ericson2314 updated this revision to Diff 460222.
Ericson2314 added a comment.

Rebase, fix some issues (by the looks of it)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130586/new/

https://reviews.llvm.org/D130586

Files:
  clang/CMakeLists.txt
  clang/cmake/caches/Android-stage2.cmake
  clang/cmake/caches/Android.cmake
  clang/cmake/modules/AddClang.cmake
  clang/lib/Headers/CMakeLists.txt
  clang/runtime/CMakeLists.txt
  clang/tools/libclang/CMakeLists.txt
  clang/tools/scan-build-py/CMakeLists.txt
  cmake/Modules/GNUInstallPackageDir.cmake
  cmake/Modules/LLVMLibdirSuffix.cmake
  compiler-rt/cmake/Modules/CompilerRTAIXUtils.cmake
  compiler-rt/cmake/Modules/CompilerRTUtils.cmake
  compiler-rt/cmake/base-config-ix.cmake
  compiler-rt/docs/BuildingCompilerRT.rst
  flang/cmake/modules/AddFlang.cmake
  libc/CMakeLists.txt
  libc/lib/CMakeLists.txt
  libcxx/CMakeLists.txt
  libcxx/docs/BuildingLibcxx.rst
  libcxxabi/CMakeLists.txt
  libunwind/CMakeLists.txt
  libunwind/docs/BuildingLibunwind.rst
  lld/cmake/modules/AddLLD.cmake
  lldb/cmake/modules/AddLLDB.cmake
  lldb/cmake/modules/LLDBGenerateConfig.cmake
  lldb/source/API/CMakeLists.txt
  lldb/tools/intel-features/CMakeLists.txt
  llvm/CMakeLists.txt
  llvm/cmake/modules/AddLLVM.cmake
  llvm/cmake/modules/AddOCaml.cmake
  llvm/cmake/modules/CMakeLists.txt
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/docs/CMake.rst
  llvm/tools/llvm-config/BuildVariables.inc.in
  llvm/tools/llvm-config/llvm-config.cpp
  llvm/utils/gn/secondary/llvm/tools/llvm-config/BUILD.gn
  mlir/CMakeLists.txt
  mlir/cmake/modules/AddMLIR.cmake
  mlir/cmake/modules/AddMLIRPython.cmake
  openmp/CMakeLists.txt
  openmp/README.rst
  polly/CMakeLists.txt
  polly/cmake/CMakeLists.txt
  polly/cmake/polly_macros.cmake
  pstl/CMakeLists.txt
  third-party/benchmark/src/CMakeLists.txt

Index: third-party/benchmark/src/CMakeLists.txt
===
--- third-party/benchmark/src/CMakeLists.txt
+++ third-party/benchmark/src/CMakeLists.txt
@@ -79,7 +79,7 @@
 configure_package_config_file (
   ${PROJECT_SOURCE_DIR}/cmake/Config.cmake.in
   ${project_config}
-  INSTALL_DESTINATION ${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}
+  INSTALL_DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/${PROJECT_NAME}"
   NO_SET_AND_CHECK_MACRO
   NO_CHECK_REQUIRED_COMPONENTS_MACRO
 )
@@ -100,8 +100,8 @@
   install(
 TARGETS ${targets_to_export}
 EXPORT ${targets_export_name}
-ARCHIVE DESTINATION ${CMAKE_INSTALL_LIBDIR}
-LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
+ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}"
+LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
 RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
 INCLUDES DESTINATION ${CMAKE_INSTALL_INCLUDEDIR})
 
Index: pstl/CMakeLists.txt
===
--- pstl/CMakeLists.txt
+++ pstl/CMakeLists.txt
@@ -88,10 +88,10 @@
 install(EXPORT ParallelSTLTargets
 FILE ParallelSTLTargets.cmake
 NAMESPACE pstl::
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL")
 install(FILES "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfig.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/ParallelSTLConfigVersion.cmake"
-DESTINATION lib/cmake/ParallelSTL)
+DESTINATION "${CMAKE_INSTALL_LIBDIR}/cmake/ParallelSTL")
 install(DIRECTORY include/
 DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}"
 PATTERN "*.in" EXCLUDE)
Index: polly/cmake/polly_macros.cmake
===
--- polly/cmake/polly_macros.cmake
+++ polly/cmake/polly_macros.cmake
@@ -44,8 +44,8 @@
   if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY OR ${name} STREQUAL "LLVMPolly")
 install(TARGETS ${name}
   EXPORT LLVMExports
-  LIBRARY DESTINATION lib${LLVM_LIBDIR_SUFFIX}
-  ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX})
+  LIBRARY DESTINATION "${CMAKE_INSTALL_LIBDIR}"
+  ARCHIVE DESTINATION "${CMAKE_INSTALL_LIBDIR}")
   endif()
   set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${name})
 endmacro(add_polly_library)
Index: polly/cmake/CMakeLists.txt
===
--- polly/cmake/CMakeLists.txt
+++ polly/cmake/CMakeLists.txt
@@ -7,7 +7,7 @@
 set(POLLY_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/polly" CACHE STRING
   "Path for CMake subdirectory for Polly (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/polly')")
 # CMAKE_INSTALL_PACKAGEDIR might be absolute, so don't reuse below.
-set(polly_cmake_builddir "${POLLY_BINARY_DIR}/lib${LLVM_LIBDIR_SUFFIX}/cmake/polly")
+set(polly_cmake_builddir "${POLLY_BINARY_LIBDIR}/cmake/polly")
 
 set(LLVM_INSTALL_PACKAGE_DIR "${CMAKE_INSTALL_PACKAGEDIR}/llvm" CACHE STRING
   "Path for CMake subdirectory for LLVM (defaults to '${CMAKE_INSTALL_PACKAGEDIR}/llvm')")
@@ -61,7 +61,7 @@
 set(POLLY_CONFIG_INCLUDE_DIRS