Page MenuHomePhabricator

aprantl (Adrian Prantl)
User

Projects

User Details

User Since
Mar 2 2013, 8:12 AM (341 w, 5 d)

Recent Activity

Yesterday

aprantl added a comment to D67563: Debug Info: Add support for named constants.

How do Fortran compilers represent regular versus inout arguments in DWARF?

Thu, Sep 19, 8:48 AM · Restricted Project, debug-info
aprantl added a comment to D67563: Debug Info: Add support for named constants.

How do Fortran compilers represent regular versus inout arguments in DWARF?

Thu, Sep 19, 8:36 AM · Restricted Project, debug-info
aprantl added a comment to D67563: Debug Info: Add support for named constants.

Swift's let is indeed closer to C's const than to a named constant in Pascal (https://wiki.freepascal.org/Constants). The Swift language spec call them constants, and they are effectively SSA variables (with storage). But (and this is where it differs from Pascal constants) let bindings can also appear inside of composite types (there are let struct and class members) and as function arguments (unless a parameter is explicitly marked as inout, similar to Fortran). I have found that DW_TAG_const is the only mechanism that works for all of these use-cases.

Thu, Sep 19, 8:35 AM · Restricted Project, debug-info

Wed, Sep 18

aprantl committed rG0779dffbd4a9: Remove the obsolete BlockByRefStruct flag from LLVM IR (authored by aprantl).
Remove the obsolete BlockByRefStruct flag from LLVM IR
Wed, Sep 18, 3:39 PM
aprantl abandoned D67563: Debug Info: Add support for named constants.

After some consideration I concluded that this isn't the best approach either. [There's no plan to do that, but] If we ever want to serialize Swift types into DWARF then an attribute on (local) variables isn't enough to describe let-members of structs. So instead I'm currently preparing a patch (no LLVM involved) that will wrap let-bound types in DW_TAG_const_type (like the const qualifier in C).

Wed, Sep 18, 2:39 PM · Restricted Project, debug-info
aprantl accepted D67613: [DWARF-5] Support for DWARF-5 C++ language tags.

Please be sure the watch the lldb (and other debugger) bots after committing this..

Wed, Sep 18, 2:24 PM · Restricted Project, debug-info
aprantl accepted D67703: Insertion of unconditional branch during FastISel differs when debug information is enabled.
Wed, Sep 18, 2:22 PM · Restricted Project

Tue, Sep 17

aprantl added a comment to D66121: Debug Info: Nest Objective-C property function decls inside their container..

I'm afraid I'm going to give up on fixing the AST and return to my debuginfo-only patch.

Tue, Sep 17, 2:28 PM · debug-info
aprantl added a comment to D66121: Debug Info: Nest Objective-C property function decls inside their container..
Tue, Sep 17, 2:21 PM · debug-info
aprantl added inline comments to D67666: GSYM: Add the llvm::gsym::Header header class with tests.
Tue, Sep 17, 10:05 AM · Restricted Project
aprantl added inline comments to D67613: [DWARF-5] Support for DWARF-5 C++ language tags.
Tue, Sep 17, 10:02 AM · Restricted Project, debug-info
aprantl added inline comments to D67613: [DWARF-5] Support for DWARF-5 C++ language tags.
Tue, Sep 17, 8:26 AM · Restricted Project, debug-info

Mon, Sep 16

aprantl added inline comments to D67613: [DWARF-5] Support for DWARF-5 C++ language tags.
Mon, Sep 16, 9:56 AM · Restricted Project, debug-info

Fri, Sep 13

aprantl added a comment to D67563: Debug Info: Add support for named constants.

Would seem to me (with an obvious C++ bias in my view of the world) that it maps best to i_am_a_constant being a DW_TAG_variable (or function_parameter) with a DW_TAG_const_type that has a base type of "Int" (however that's represented).

But I realize that might not be a good fit for Swift/its debuggers, etc, and certainly tend to leave this up to whatever you folks want to do with your language. Just my 2c :)

Fri, Sep 13, 5:02 PM · Restricted Project, debug-info
aprantl added a comment to D67563: Debug Info: Add support for named constants.

-> https://docs.swift.org/swift-book/ReferenceManual/Types.html

Fri, Sep 13, 4:41 PM · Restricted Project, debug-info
aprantl added a comment to D67563: Debug Info: Add support for named constants.

Out of curiosity does swift have const in the type system like C and C++? (I guess so) - and what would be the difference between a normal (non-let) variable of a const type, versus a let variable of a non-const type?

Fri, Sep 13, 4:41 PM · Restricted Project, debug-info
aprantl updated the diff for D67563: Debug Info: Add support for named constants.

Okay. Here is an alternative patch that implements a flag DW_AT_LLVM_constant. "The Swift Programming Language" calls let-bindings "constants" and constant seems to be a generic enough term to be generally useful.

Fri, Sep 13, 4:06 PM · Restricted Project, debug-info
aprantl added a comment to D67500: [DebugInfo] LiveDebugValues: don't create transfer records for potentially invalid locations.

I'm assuming that at the beginning all in-locations are unknown and only when all incoming basic blocks have outgoing locations, join() adds new incoming locations to be processed.

Ah, this is where the implementation diverges, in fact if all incoming blocks have the same outgoing location *or* unknown for a variable, join() will consider it a valid live-in location. I think it has to be this way because of loops -- for a location that is live throughout a loop, there's a circular dependency between the outgoing location for the back-edge and the live-in location for the loop header. Right now we assume the location is correct for any unknown blocks, and have to invalidate locations later if that doesn't turn out to be true. See for example D66599.

I guess this boils down to how we are not really modeling "unknown" currently?

It turns out to be even more exciting than that -- consider this IR [0] which has badly behaved, but completely legal control flow. It's illustrated in this highly sophisticated diagram [1]. A single variable location (toast==0) is created in the entry block, and then invalidated with an undef dbg.value in loop 3. Because every non-entry/non-exit block is reachable from every other block, the location-invalidation has to be propagated to them all. However, it takes at least one LiveDebugValues iteration for the invalidation to traverse a back-edge, so it takes two LiveDebugvalues iterations for the invalidation to move up the CFG and reach loop1. In that time, all the "unknown" blocks have been processed, and as a result locations have to transition from "true" to "false", rather than "unknown" to "false". It might be possible to generate a situation where something transitions from "false" to "true" too, but I haven't looked closely at that yet.

I think the fact that this is happening means I've made LiveDebugValues depart from peoples expectations, you wrote in the 2016 mailing list dicsussion [2]

the information at the nodes can only move in one direction in the lattice and can change at most once.

which isn't true any more because of D66599 removing locations. I suspect I've turned LiveDebugValues into a data-flow-like algorithm rather than a joining-lattices algorithm. Whether this is a good idea or not probably needs discussing; it's all been in aid of getting rid of invalid locations though.

[0] https://reviews.llvm.org/P8163
[1] https://imgur.com/a/YdE9kN7
[2] https://groups.google.com/forum/#!search/llvm-dev/llvm-dev/nlgxIfDQnhs/4zsGbuoTAAAJ

Fri, Sep 13, 3:02 PM · Restricted Project
aprantl added a comment to D67563: Debug Info: Add support for named constants.

Before creating this patch I had considered 4 options:

Fri, Sep 13, 2:40 PM · Restricted Project, debug-info
aprantl added a child revision for D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR: D67563: Debug Info: Add support for named constants.
Fri, Sep 13, 10:33 AM · Restricted Project, debug-info
aprantl added a parent revision for D67563: Debug Info: Add support for named constants: D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR.
Fri, Sep 13, 10:33 AM · Restricted Project, debug-info
aprantl created D67563: Debug Info: Add support for named constants.
Fri, Sep 13, 10:33 AM · Restricted Project, debug-info
aprantl added inline comments to D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation.
Fri, Sep 13, 8:27 AM · Restricted Project, debug-info

Thu, Sep 12

aprantl added a comment to D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation.

Normally I would insist on a bitcode upgrade, but since this feature is so new I think we can skip that. However, because of bitcode compatability, we might want to get the semantics right from the start. Otherwise we might have to allocate a new opcode and implement an upgrade when we need to change it in the future.

Thu, Sep 12, 3:08 PM · Restricted Project, debug-info
aprantl accepted D67506: GSYM: add encoding and decoding to FunctionInfo.

minor nits inline

Thu, Sep 12, 3:03 PM
aprantl added a comment to D67506: GSYM: add encoding and decoding to FunctionInfo.

Looks mostly good.

Thu, Sep 12, 10:40 AM
aprantl added a comment to D67500: [DebugInfo] LiveDebugValues: don't create transfer records for potentially invalid locations.

just don't record transfers until the in-locations are all known to be correct

Thu, Sep 12, 10:35 AM · Restricted Project
aprantl added a comment to D67492: [DebugInfo] Add a DW_OP_LLVM_entry_value operation.

Having suggested this, I think this is a good idea ;-)

Thu, Sep 12, 10:26 AM · Restricted Project, debug-info

Wed, Sep 11

aprantl updated the diff for D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR.

Found one more stale decl.

Wed, Sep 11, 3:59 PM · Restricted Project, debug-info
aprantl updated the diff for D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR.

Good point!

Wed, Sep 11, 3:48 PM · Restricted Project, debug-info
aprantl added a comment to D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR.

My plan is to reuse the bit for an attribute that does not appear inside of DICompositeType, so we could leave the Verifier check in indefinitely. I should have said so though.

Wed, Sep 11, 1:17 PM · Restricted Project, debug-info
aprantl accepted D67376: [DWARF] Evaluate DW_OP_entry_value.

Thanks, that's exactly what I was looking for!

Wed, Sep 11, 1:17 PM · Restricted Project
aprantl accepted D66602: Add a LineTable class to GSYM and test it..

Thanks!

Wed, Sep 11, 1:01 PM
aprantl committed rG09f320ad3415: Update link to the DWARF spec. (authored by aprantl).
Update link to the DWARF spec.
Wed, Sep 11, 12:58 PM
aprantl committed rG4a5dd4a88165: Update documentation. (authored by aprantl).
Update documentation.
Wed, Sep 11, 12:49 PM
aprantl created D67453: Remove the obsolete BlockByRefStruct flag from LLVM IR.
Wed, Sep 11, 11:33 AM · Restricted Project, debug-info
aprantl added a comment to D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.

Otherwise this LGTM now.

Wed, Sep 11, 9:37 AM
aprantl added inline comments to D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
Wed, Sep 11, 9:34 AM
aprantl added inline comments to D66602: Add a LineTable class to GSYM and test it..
Wed, Sep 11, 9:32 AM
aprantl added a comment to D67410: [DWARF] Emit call site parameter info when tuning for lldb.

I'm very curious what the effect of this patch on our green dragon statistics bot will be (http://green.lab.llvm.org/green/view/LLDB/job/clang-3.4-debuginfo-statistics/).

Wed, Sep 11, 9:15 AM · debug-info, Restricted Project
aprantl added a comment to D67410: [DWARF] Emit call site parameter info when tuning for lldb.

An idea: After this got implemented, we can add a new sub-directory inside the debuginfo-tests project called DOC (Debugging Optimized Code) and start testing (and tracking) improvements in that area. WDYT?

Wed, Sep 11, 9:09 AM · debug-info, Restricted Project
aprantl added inline comments to D67376: [DWARF] Evaluate DW_OP_entry_value.
Wed, Sep 11, 9:01 AM · Restricted Project
aprantl added a comment to D67376: [DWARF] Evaluate DW_OP_entry_value.

Looking great! I have one more question inline, but that is mostly about documenting the algorithm.

Wed, Sep 11, 9:00 AM · Restricted Project
aprantl added inline comments to D67217: [Debuginfo][Instcombiner] Do not clone dbg.declare in TryToSinkInstruction().
Wed, Sep 11, 8:47 AM · Restricted Project, debug-info
aprantl added a comment to D67393: [DebugInfo] LiveDebugValues: Defer all DBG_VALUE creation during analysis.

Okay, we should make sure that we do get the design right. I don't quite get the argument made in SourceLevelDebugInfo.rst for why the semantics are different:

Wed, Sep 11, 8:38 AM · Restricted Project

Tue, Sep 10

aprantl added a comment to D67393: [DebugInfo] LiveDebugValues: Defer all DBG_VALUE creation during analysis.

As stated in the docs here [0], the meaning of a DBG_VALUE instruction changes after LiveDebugValues runs

Tue, Sep 10, 4:19 PM · Restricted Project
aprantl added a comment to D66121: Debug Info: Nest Objective-C property function decls inside their container..

Can you prepare an NFC patch with just the changes relating to adding ObjCPropertyImplDecl::get{Getter,Setter}MethodDecl?

Tue, Sep 10, 4:16 PM · debug-info
aprantl added inline comments to D67376: [DWARF] Evaluate DW_OP_entry_value.
Tue, Sep 10, 3:52 PM · Restricted Project
aprantl added inline comments to D67111: Adding caching to libc++ std::function formatter for lookups that require scanning symbols.
Tue, Sep 10, 3:30 PM
aprantl accepted D67410: [DWARF] Emit call site parameter info when tuning for lldb.
Tue, Sep 10, 3:21 PM · debug-info, Restricted Project
aprantl committed rG9b23df63ecd9: Implement DW_OP_convert (authored by aprantl).
Implement DW_OP_convert
Tue, Sep 10, 9:19 AM
aprantl added inline comments to D67369: Implement DW_OP_convert.
Tue, Sep 10, 9:08 AM · Restricted Project, Restricted Project
aprantl added inline comments to D67369: Implement DW_OP_convert.
Tue, Sep 10, 8:55 AM · Restricted Project, Restricted Project
aprantl committed rGf987f561fadc: Don't emit .gnu_pubnames when tuning for LLDB. (authored by aprantl).
Don't emit .gnu_pubnames when tuning for LLDB.
Tue, Sep 10, 8:53 AM
aprantl added inline comments to D67398: [DebugInfo] LiveDebugValues: Move DBG_VALUE creation into VarLoc class.
Tue, Sep 10, 8:50 AM · Restricted Project
aprantl added inline comments to D67376: [DWARF] Evaluate DW_OP_entry_value.
Tue, Sep 10, 8:41 AM · Restricted Project

Mon, Sep 9

aprantl added inline comments to D67369: Implement DW_OP_convert.
Mon, Sep 9, 4:34 PM · Restricted Project, Restricted Project
aprantl added inline comments to D67369: Implement DW_OP_convert.
Mon, Sep 9, 4:11 PM · Restricted Project, Restricted Project
aprantl added a comment to D67373: Don't emit .gnu_pubnames when tuning for LLDB.

Have you got a link to the original thread where this was discussed/I mentioned it? Want to page in some context to double-check if I had any ideas that might've let this simplify things.

Mon, Sep 9, 4:10 PM · Restricted Project, Restricted Project
aprantl added inline comments to D67369: Implement DW_OP_convert.
Mon, Sep 9, 3:59 PM · Restricted Project, Restricted Project
aprantl added a comment to D67376: [DWARF] Evaluate DW_OP_entry_value.

This is very exciting!

Mon, Sep 9, 3:59 PM · Restricted Project
aprantl added a reviewer for D67369: Implement DW_OP_convert: vsk.
Mon, Sep 9, 3:59 PM · Restricted Project, Restricted Project
aprantl added inline comments to D66602: Add a LineTable class to GSYM and test it..
Mon, Sep 9, 3:28 PM
aprantl created D67373: Don't emit .gnu_pubnames when tuning for LLDB.
Mon, Sep 9, 3:19 PM · Restricted Project, Restricted Project
aprantl accepted D67370: Fix ELF core file memory reading for PT_LOAD program headers with no p_filesz.
Mon, Sep 9, 2:26 PM · Restricted Project
aprantl requested changes to D67307: [MemorySSA] Do not create memoryaccesses for debug info intrinsics..
Mon, Sep 9, 2:24 PM · Restricted Project
aprantl added inline comments to D67307: [MemorySSA] Do not create memoryaccesses for debug info intrinsics..
Mon, Sep 9, 2:24 PM · Restricted Project
aprantl accepted D67307: [MemorySSA] Do not create memoryaccesses for debug info intrinsics..
Mon, Sep 9, 2:24 PM · Restricted Project
aprantl added inline comments to D67263: [DwarfExpression] Disallow some rewrites to avoid undefined behavior.
Mon, Sep 9, 2:14 PM · Restricted Project, debug-info
aprantl accepted D66526: [utils] Add the llvm-locstats tool.
Mon, Sep 9, 2:07 PM · Restricted Project, debug-info
aprantl added inline comments to D67217: [Debuginfo][Instcombiner] Do not clone dbg.declare in TryToSinkInstruction().
Mon, Sep 9, 2:04 PM · Restricted Project, debug-info
aprantl accepted D67217: [Debuginfo][Instcombiner] Do not clone dbg.declare in TryToSinkInstruction().

This is a strict improvement and as such is fine, but there's some general cleanups that are uncovered by this that we should also be doing.

Mon, Sep 9, 2:04 PM · Restricted Project, debug-info
aprantl added a reviewer for D67369: Implement DW_OP_convert: jakubjelinek.
Mon, Sep 9, 1:39 PM · Restricted Project, Restricted Project
aprantl created D67369: Implement DW_OP_convert.
Mon, Sep 9, 1:39 PM · Restricted Project, Restricted Project

Fri, Sep 6

aprantl added inline comments to D67307: [MemorySSA] Do not create memoryaccesses for debug info intrinsics..
Fri, Sep 6, 4:24 PM · Restricted Project

Thu, Sep 5

aprantl added a comment to D67249: [Modules][PCH] Hash input files content.

Nice! Are you planning to address the FIXME's in a later update of this patch?

Thu, Sep 5, 6:55 PM · Restricted Project

Wed, Sep 4

aprantl added a comment to D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..

Enjoyable leaps in variable-coverage / bytes-coverage on the far right of these graphs:

http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607043.4&highlight_run=128792
http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607042.4&highlight_run=128792

Thanks for digging into this!

Wed, Sep 4, 12:52 PM · Restricted Project, debug-info
aprantl accepted D66941: [DebugInfo] LiveDebugValues: explicitly terminate overwritten stack locations.

I see we would have to do an abstract interpretation of a DIExpression together with the implicit register operand bound by the DBG_VALUE to computes all stack slots that my be touched by the expression, assuming we even know where non-frame-pointer registers point. Could you add an explanation to the comment that lays out why this isn't feasible?

Wed, Sep 4, 12:48 PM · Restricted Project
aprantl committed rG2461061168f4: Upstream macCatalyst support in debugserver and the macOS dynamic loader plugin. (authored by aprantl).
Upstream macCatalyst support in debugserver and the macOS dynamic loader plugin.
Wed, Sep 4, 10:24 AM

Tue, Sep 3

aprantl added inline comments to D46791: Make -gsplit-dwarf generally available.
Tue, Sep 3, 1:11 PM
aprantl created D67124: Upstream macCatalyst support in debugserver and the dynamic library loader plugin..
Tue, Sep 3, 12:44 PM · Restricted Project
aprantl accepted D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..
Tue, Sep 3, 10:59 AM · Restricted Project, debug-info
aprantl added inline comments to D67083: [dotest] Avoid the need for LEVEL= makefile boilerplate.
Tue, Sep 3, 10:15 AM · Restricted Project
aprantl accepted D67097: [DWARF] Check for format mismatch between CU and Range List Table..
Tue, Sep 3, 9:28 AM · Restricted Project, debug-info
aprantl accepted D67098: [DWARF] Fix referencing Range List Tables from CUs for DWARF64..
Tue, Sep 3, 9:27 AM · Restricted Project, debug-info
aprantl accepted D67083: [dotest] Avoid the need for LEVEL= makefile boilerplate.

This looks like a very good idea!

Tue, Sep 3, 9:27 AM · Restricted Project
aprantl added inline comments to D66941: [DebugInfo] LiveDebugValues: explicitly terminate overwritten stack locations.
Tue, Sep 3, 9:23 AM · Restricted Project
aprantl accepted D66942: [DebugInfo] LiveDebugValues: locations with different exprs should compare differently.
Tue, Sep 3, 9:18 AM · Restricted Project
aprantl accepted D58386: [DebugInfo] Pre-RA MachineSink: sink DBG_VALUEs that don't immediately follow the sunk instruction too.
Tue, Sep 3, 9:12 AM · Restricted Project
aprantl added inline comments to D58238: [DebugInfo] MachineSink: Insert undef DBG_VALUEs when sinking instructions, try to forward copies.
Tue, Sep 3, 9:06 AM · Restricted Project
aprantl added inline comments to D67004: [DebugInfo] Enable call site parameter debug info for ARM and AArch64.
Tue, Sep 3, 9:06 AM · debug-info
aprantl added a comment to D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..

lgtm with oustanding comments addressed!

Tue, Sep 3, 9:06 AM · Restricted Project, debug-info
aprantl added inline comments to D64595: [Debuginfo][SROA] Need to handle dbg.value in SROA pass..
Tue, Sep 3, 9:06 AM · Restricted Project, debug-info
aprantl accepted D66600: Add encode and decode methods to InlineInfo and document encoding format to the GSYM file format.

Thanks for implementing the error handling! There is one more comment inline.

Tue, Sep 3, 9:00 AM

Thu, Aug 29

aprantl updated the diff for D66121: Debug Info: Nest Objective-C property function decls inside their container..

Here is a work-in-progress alternative patch that attacks the problem by changing the AST generation to have an ObjCMethodDecl for the property accessors inside the implementation as suggested by @rjmccall.

Thu, Aug 29, 5:04 PM · debug-info
aprantl added inline comments to D66955: [DebugInfo][If-Converter] Update call site info during the optimization.
Thu, Aug 29, 4:18 PM · debug-info
aprantl accepted D66975: [lit] Print exit code in for unresolved (lldb)tests..

If it helps...
Is there anything else we can print for a crashed dotest?

Thu, Aug 29, 2:51 PM · Restricted Project, Restricted Project
aprantl added inline comments to D66955: [DebugInfo][If-Converter] Update call site info during the optimization.
Thu, Aug 29, 2:48 PM · debug-info
aprantl added a comment to D66942: [DebugInfo] LiveDebugValues: locations with different exprs should compare differently.

This makes sense, I'm kind of shocked that me missed something that obvious. Is there a big drop in variable locations with this (i.e., an improvement in accuracy)?

Thu, Aug 29, 2:47 PM · Restricted Project
aprantl added a comment to D66941: [DebugInfo] LiveDebugValues: explicitly terminate overwritten stack locations.

Currently, if a stack spill location is overwritten to by another spill instruction, nothing notices and any variables located in that slot are not terminated.

Thu, Aug 29, 2:22 PM · Restricted Project