[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls

2022-06-28 Thread Eli Friedman via Phabricator via lldb-commits
efriedma added a comment.

A couple notes skimming the changes:

1. If you're going to update URLs, please verify the new URLs are actually 
valid.
2. Some of the "URLs" you're updating aren't actually URLs. For example, the 
`xmlns` attribute in SVG files must be the exact string written in the 
standard; changing it could make the file unreadable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127684

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


[Lldb-commits] [PATCH] D129131: Remove uses of llvm_shutdown

2022-08-03 Thread Eli Friedman via Phabricator via lldb-commits
efriedma added inline comments.



Comment at: polly/lib/External/isl/interface/extract_interface.cc:590
delete Clang;
-   llvm::llvm_shutdown();
 

nhaehnle wrote:
> Meinersbur wrote:
> > This file is imported from the upstream project 
> > (https://repo.or.cz/isl.git/blob/295cf91923295ca694e9e4433a0b818150421bee:/interface/extract_interface.cc#l590)
> >  and this change will be lost when synchronizing with it. However, this 
> > file is also not used within LLVM. I recommend to just keep as-is.
> Thanks, will do.
I think you missed the other change in this file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129131

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


[Lldb-commits] [PATCH] D140996: [c++20] P1907R1: Support for generalized non-type template arguments of scalar type.

2023-05-14 Thread Eli Friedman via Phabricator via lldb-commits
efriedma added inline comments.



Comment at: clang/lib/AST/ItaniumMangle.cpp:4579
+  if (CE->hasAPValueResult())
+mangleValueInTemplateArg(ParamType, CE->getResultAsAPValue(), false,
+ /*NeedExactType=*/true);

I'm not sure what the point of the `if (CE->hasAPValueResult())` is; are you 
just trying to avoid copying the APValue?  (If this is going to be a repeating 
pattern, maybe we can add some sort of utility class to represent the pattern.)



Comment at: clang/lib/AST/ItaniumMangle.cpp:4397
+// argument.
+// As proposed in https://github.com/itanium-cxx-abi/cxx-abi/issues/111.
+auto *SNTTPE = cast(E);

bolshakov-a wrote:
> erichkeane wrote:
> > erichkeane wrote:
> > > aaron.ballman wrote:
> > > > We should get this nailed down. It was proposed in Nov 2020 and the 
> > > > issue is still open. CC @rjmccall 
> > > This definitely needs to happen.  @rjmccall or @eli.friedman ^^ Any idea 
> > > what the actual mangling should be?
> > This is still an open, and we need @rjmccall @eli.friedman or @asl to help 
> > out here.
> Ping @efriedma, @rjmccall, @asl.
I'm not really familiar with the mangling implications for this particular 
construct, nor am I actively involved with the Itanium ABI specification, so 
I'm not sure how I can help you directly.

That said, as a general opinion, I don't think it's worth waiting for updates 
to the Itanuim ABI  document to be merged; such updates are happening slowly at 
the moment, and having a consistent mangling is clearly an improvement even if 
it's not specified.  My suggested plan of action:

- Make sure you're satisfied the proposed mangling doesn't have any holes 
you're concerned about (i.e. it produces a unique mangling for all the relevant 
cases).  If you're not sure, I can try to spend some time understanding this, 
but it doesn't sound like you have any concerns about this.
- Put a note on the issue in the Itanium ABI repo that you're planning to go 
ahead with using this mangling in clang.  Also send an email directly to 
@rjmccall and @rsmith in case they miss the notifications.
- Go ahead with this.


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

https://reviews.llvm.org/D140996

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


[Lldb-commits] [PATCH] D148761: Emit the correct flags for the PROC CodeView Debug Symbol

2023-05-16 Thread Eli Friedman via Phabricator via lldb-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf8499d5709e3: Emit the correct flags for the PROC CodeView 
Debug Symbol (authored by dpaoliello, committed by efriedma).
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Changed prior to commit:
  https://reviews.llvm.org/D148761?vs=522303&id=522711#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148761

Files:
  lldb/test/Shell/SymbolFile/PDB/function-nested-block.test
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
  llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.h
  llvm/test/DebugInfo/COFF/fpo-realign-alloca.ll
  llvm/test/DebugInfo/COFF/fpo-realign-vframe.ll
  llvm/test/DebugInfo/COFF/frameproc-flags.ll
  llvm/test/DebugInfo/COFF/function-options.ll
  llvm/test/DebugInfo/COFF/inlining-header.ll
  llvm/test/DebugInfo/COFF/inlining.ll
  llvm/test/DebugInfo/COFF/long-name.ll
  llvm/test/DebugInfo/COFF/multifunction.ll
  llvm/test/DebugInfo/COFF/simple.ll
  llvm/test/DebugInfo/COFF/types-array.ll
  llvm/test/DebugInfo/COFF/types-basic.ll
  llvm/test/MC/AArch64/coff-debug.ll

Index: llvm/test/MC/AArch64/coff-debug.ll
===
--- llvm/test/MC/AArch64/coff-debug.ll
+++ llvm/test/MC/AArch64/coff-debug.ll
@@ -95,7 +95,9 @@
 ; CHECK:   FunctionType: main (0x1002)
 ; CHECK:   CodeOffset: main+0x0
 ; CHECK:   Segment: 0x0
-; CHECK:   Flags [ (0x0)
+; CHECK:   Flags [ (0xC0)
+; CHECK: HasOptimizedDebugInfo (0x80)
+; CHECK: IsNoInline (0x40)
 ; CHECK:   ]
 ; CHECK:   DisplayName: main
 ; CHECK:   LinkageName: main
Index: llvm/test/DebugInfo/COFF/types-basic.ll
===
--- llvm/test/DebugInfo/COFF/types-basic.ll
+++ llvm/test/DebugInfo/COFF/types-basic.ll
@@ -221,7 +221,8 @@
 ; CHECK:   FunctionType: f (0x1002)
 ; CHECK:   CodeOffset: ?f@@YAXMN_J@Z+0x0
 ; CHECK:   Segment: 0x0
-; CHECK:   Flags [ (0x0)
+; CHECK:   Flags [ (0x80)
+; CHECK: HasOptimizedDebugInfo (0x80)
 ; CHECK:   ]
 ; CHECK:   DisplayName: f
 ; CHECK:   LinkageName: ?f@@YAXMN_J@Z
Index: llvm/test/DebugInfo/COFF/types-array.ll
===
--- llvm/test/DebugInfo/COFF/types-array.ll
+++ llvm/test/DebugInfo/COFF/types-array.ll
@@ -58,7 +58,9 @@
 ; CHECK:   FunctionType: f (0x1002)
 ; CHECK:   CodeOffset: ?f@@YAXXZ+0x0
 ; CHECK:   Segment: 0x0
-; CHECK:   Flags [ (0x0)
+; CHECK:   Flags [ (0x81)
+; CHECK: HasFP (0x1)
+; CHECK: HasOptimizedDebugInfo (0x80)
 ; CHECK:   ]
 ; CHECK:   DisplayName: f
 ; CHECK:   LinkageName: ?f@@YAXXZ
Index: llvm/test/DebugInfo/COFF/simple.ll
===
--- llvm/test/DebugInfo/COFF/simple.ll
+++ llvm/test/DebugInfo/COFF/simple.ll
@@ -58,7 +58,7 @@
 ; X86-NEXT: .long   4098
 ; X86-NEXT: .secrel32 _f
 ; X86-NEXT: .secidx _f
-; X86-NEXT: .byte   0
+; X86-NEXT: .byte   128
 ; X86-NEXT: .asciz "f"
 ; X86-NEXT: .p2align 2
 ; X86-NEXT: [[PROC_SEGMENT_END]]:
@@ -188,7 +188,7 @@
 ; X64-NEXT: .long   4098
 ; X64-NEXT: .secrel32 f
 ; X64-NEXT: .secidx f
-; X64-NEXT: .byte   0
+; X64-NEXT: .byte   128
 ; X64-NEXT: .asciz "f"
 ; X64-NEXT: .p2align 2
 ; X64-NEXT: [[PROC_SEGMENT_END]]:
Index: llvm/test/DebugInfo/COFF/multifunction.ll
===
--- llvm/test/DebugInfo/COFF/multifunction.ll
+++ llvm/test/DebugInfo/COFF/multifunction.ll
@@ -78,7 +78,7 @@
 ; X86-NEXT: .long   4098
 ; X86-NEXT: .secrel32 _x
 ; X86-NEXT: .secidx _x
-; X86-NEXT: .byte   0
+; X86-NEXT: .byte   128
 ; X86-NEXT: .asciz "x"
 ; X86-NEXT: .p2align 2
 ; X86-NEXT: [[PROC_SEGMENT_END]]:
@@ -117,7 +117,7 @@
 ; X86-NEXT: .long   4099
 ; X86-NEXT: .secrel32 _y
 ; X86-NEXT: .secidx _y
-; X86-NEXT: .byte   0
+; X86-NEXT: .byte   128
 ; X86-NEXT: .asciz "y"
 ; X86-NEXT: .p2align 2
 ; X86-NEXT: [[PROC_SEGMENT_END]]:
@@ -156,7 +156,7 @@
 ; X86-NEXT: .long   4100
 ; X86-NEXT: .secrel32 _f
 ; X86-NEXT: .secidx _f
-; X86-NEXT: .byte   0
+; X86-NEXT: .byte   128
 ; X86-NEXT: .asciz "f"
 ; X86-NEXT: .p2align 2
 ; X86-NEXT: [[PROC_SEGMENT_END]]:
@@ -390,7 +390,7 @@
 ; X64-NEXT: .long   4098
 ; X64-NEXT: .secrel32 x
 ; X64-NEXT: .secidx x
-; X64-NEXT: .byte   0
+; X64-NEXT: .byte   128
 ; X64-NEXT: .asciz "x"
 ; X64-NEXT: .p2align 2
 ; X64-NEXT: [[PROC_SEGMENT_END]]:
@@ -428,7 +428,7 @@
 ; X64-NEXT: .long   4099
 ; X64-NEXT: .secrel32 y
 ; X64-NEXT: .secidx y
-; X64-NEXT: .byte   0
+; X64-NEXT: .byte   128
 ; X64-NEXT: .asciz "y"
 ; X64-NEXT: .p2align 2
 ; X64-NEXT: [[PROC_SEGMENT_END]]:
@@ -466,7 +466,7 @@
 ; X64-NEXT: .long   4100
 ; X64-NEXT: .secrel32 f
 ; X64-NEXT: .secidx f
-; X64-NEXT: .byte   0
+; X64-NEXT: .byte   128
 ; X64-NEXT: .asciz "f"
 ; X64-NEXT: .p2align

[Lldb-commits] [PATCH] D151003: [Damangle] convert dlangDemangle to use std::string_view

2023-06-05 Thread Eli Friedman via Phabricator via lldb-commits
efriedma added inline comments.



Comment at: llvm/lib/Demangle/DLangDemangle.cpp:555
 
-Demangler D = Demangler(MangledName);
-MangledName = D.parseMangle(&Demangled);
+Demangler D(MangledName.data());
+const char *M = D.parseMangle(&Demangled);

Isn't this still assuming MangledName is null-terminated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151003

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


[Lldb-commits] [PATCH] D77587: [SVE] Add new VectorType subclasses

2020-04-09 Thread Eli Friedman via Phabricator via lldb-commits
efriedma added a comment.

Looks right, generally.




Comment at: llvm/include/llvm-c/Core.h:163
+   * value of enum variants after the removal of LLVMVectorTypeKind
+   */
+  LLVMMetadataTypeKind = LLVMPointerTypeKind + 2, /**< Metadata */

The C API is officially not ABI-stable anymore across versions, No reason to 
have a gap like this.



Comment at: llvm/lib/ExecutionEngine/ExecutionEngine.cpp:1011
 llvm_unreachable("Unknown constant pointer type!");
-  }
-  break;
+  } break;
 

?



Comment at: llvm/lib/Target/AMDGPU/AMDGPUPrintfRuntimeBinding.cpp:481
   }
-} else if (ArgType->getTypeID() == Type::VectorTyID) {
+} else if (ArgType->getTypeID() == Type::FixedVectorTyID) {
   Type *IType = NULL;

Please fix this to use isa<> while you're here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77587



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