[Lldb-commits] [PATCH] D127684: [NFC] Use `https` instead of `http` in the urls
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
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.
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
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
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
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