Page MenuHomePhabricator

aprantl (Adrian Prantl)
User

Projects

User Details

User Since
Mar 2 2013, 8:12 AM (307 w, 1 d)

Recent Activity

Fri, Jan 18

aprantl accepted D56793: [SelectionDAG] Updates for -dag-dump-verbose.
Fri, Jan 18, 10:12 AM

Thu, Jan 17

aprantl added inline comments to D56587: Fix sign/zero extension in Dwarf expressions..
Thu, Jan 17, 8:59 AM · debug-info

Wed, Jan 16

aprantl added inline comments to D56819: Document toolchain update policy.
Wed, Jan 16, 4:40 PM
aprantl accepted D56767: [AsmPrinter] Collapse .loc 0 0 directives.

I think we can reduce the test even more, but otherwise this should be fine.

Wed, Jan 16, 11:26 AM
aprantl added inline comments to D56767: [AsmPrinter] Collapse .loc 0 0 directives.
Wed, Jan 16, 11:26 AM
aprantl created D56798: Change TypeSystem::GetBitSize() to return an optional result..
Wed, Jan 16, 11:08 AM
aprantl added a comment to D56788: [DebugInfo][InstCombine] Prefer salvaging dbg.values over sinking them.

Seems reasonable to me.

Wed, Jan 16, 8:44 AM
aprantl added inline comments to D56767: [AsmPrinter] Collapse .loc 0 0 directives.
Wed, Jan 16, 8:18 AM
aprantl added a comment to D56767: [AsmPrinter] Collapse .loc 0 0 directives.

Thanks for the patch! The testcase looks like it would bitrot very easily. I would recommend first manually reducing the IR as much as possible and then making a MIR testcase out of it to become independent from future ISEL changes.

Wed, Jan 16, 8:15 AM
aprantl added a comment to D56589: Teach the default symbol vendor to respect module.GetSymbolFileFileSpec().

Looks like this test is failing on green dragon:

Wed, Jan 16, 8:07 AM

Tue, Jan 15

aprantl added a comment to D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC).

I changed all the autos back to full types in r351237.

Tue, Jan 15, 12:55 PM
aprantl added inline comments to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.
Tue, Jan 15, 8:52 AM
aprantl updated the diff for D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC).

Thanks Pavel, your argument of optimizing storage over interfaces makes perfect sense. Using an Optional instead.

Tue, Jan 15, 8:49 AM
aprantl added a comment to D56587: Fix sign/zero extension in Dwarf expressions..

I didn't realize you were talking about DwarfExpression, sorry . I'm not really sure what the best solution is because I don't quite understand MCLabels that well. Naively I was assuming that we would emit an MCLabel that is defined as the difference between DIE offset and .debug_info start label. Since we need to emit DIE references all over .debug_info I would start by looking at how it's done there. The fact that it is in a different section may complicate things though, I'm not sure.

Tue, Jan 15, 8:44 AM · debug-info
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

Roughly: the implementation of print used code (MachineInstr operator<<) that lives in the CodeGen library, and various tools that don't link against CodeGen include header files from the CodeGen Clang module). Without local submodule visibility, this pulls in all headers of the CodeGen Clang module, thus causing code to be generated for the dump function, which then causes the linker error.

Tue, Jan 15, 8:40 AM

Mon, Jan 14

aprantl created D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC).
Mon, Jan 14, 4:22 PM
aprantl added a comment to D56678: [DebugInfo][DAG] Allow creation of DBG_VALUEs in blocks where the operand is not used.

Seems okay to me, but it would be good to wait Björn's LGTM.

Mon, Jan 14, 12:34 PM
aprantl added a comment to D56587: Fix sign/zero extension in Dwarf expressions..

Referencing a DIE in DIExpression would need a bit of additional work. DIExpression stores its operands in an array of uint64_t, so in order to support MDNode operands we'd have to add them as actual MDNode operands. For example, expr_op_iterator could know which DIExpression operations take MDNode arguments and inject them in the right places. Slightly more complicated would be actually finding a matching DIType in the debug info that we want to point to. If we want to convert to exactly the type of the DIVariable, we can use that type, otherwise we might have to generate new DIBasicTypes on the fly.

Even though this sounds complicated, I still think that it is preferable to encode type conversions as such rather than generating really complicated expressions that happen to have the same effect in the underspecified DWARF 4 stack language.

Yes, long term it is likely the best solution.

I played a bit with trying to insert a DW_OP_convert before I came up with this patch but was clueless on how to retrieve the DIE offset of the DIType when the expression was emitted as that section (.debug_info?) hadn't been emitted yet. If I could get some useful advice on tackling that issue I can have another go at it when I get back to work tomorrow.

Mon, Jan 14, 12:28 PM · debug-info
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

I *think* I fixed it for good in in r351077.

Mon, Jan 14, 9:28 AM
aprantl added a comment to D56587: Fix sign/zero extension in Dwarf expressions..

Probably inserting a pseudo op here lowering it at a later stage when it is known if the value will reside in memory would be the right thing to do. Not sure if a DWARF 5 DW_OP_convert would be the easiest option as its argument references another DIE and it seems that would require larger infrastructure changes (but I really don't know anything about this).

Mon, Jan 14, 8:46 AM · debug-info

Fri, Jan 11

aprantl added inline comments to D56587: Fix sign/zero extension in Dwarf expressions..
Fri, Jan 11, 10:22 AM · debug-info
aprantl added a comment to D56587: Fix sign/zero extension in Dwarf expressions..

ii) It is probably not safe to assume that the consumer/debugger would set the high bits to zero in zero extension in e.g. the case that the variable has been spilled to memory.

Fri, Jan 11, 8:14 AM · debug-info
aprantl added inline comments to D56587: Fix sign/zero extension in Dwarf expressions..
Fri, Jan 11, 8:13 AM · debug-info

Thu, Jan 10

aprantl added inline comments to D56543: DWARF: Add some support for non-native directory separators.
Thu, Jan 10, 9:06 AM

Wed, Jan 9

aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

You have to use clang in order to compile with clang module support enabled. If your host compiler is GCC, you will have to build clang first and then use that clang to compile clang again using all the MODULE cmake options.

Wed, Jan 9, 9:06 AM

Tue, Jan 8

aprantl added inline comments to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".
Tue, Jan 8, 5:02 PM
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

It should not be mac-specific, but it is possible that Linux uses a different default for LLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY. Can you try again with -DLLVM_ENABLE_LOCAL_SUBMODULE_VISIBILITY=0 -DLLVM_ENABLE_MODULES=1 ?

Tue, Jan 8, 5:01 PM
aprantl updated subscribers of D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".
Tue, Jan 8, 5:00 PM
aprantl accepted D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).
Tue, Jan 8, 1:54 PM
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

Hmmm.. my fix didn't go far enough. I reverted this patch once again, so we can sort this out without pressure. As long as you make sure that compiling with -DLLVM_ENABLE_MODULES=1 works, feel free to recommit at any time!

Tue, Jan 8, 1:11 PM
aprantl updated subscribers of D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".
Tue, Jan 8, 12:43 PM
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

I committed a rather horrible workaround in r350650, but it would be great to revert that again.

Tue, Jan 8, 12:43 PM
aprantl added inline comments to D55919: [DebugInfo] Omit location list entries with empty ranges.
Tue, Jan 8, 8:15 AM · debug-info
aprantl added inline comments to D56425: [BreakpointList] Simplify/modernize BreakpointList (NFC).
Tue, Jan 8, 8:13 AM

Mon, Jan 7

aprantl accepted D56346: [dsymutil] Update unobfuscation logic..
Mon, Jan 7, 2:55 PM
aprantl requested changes to D56414: [Verifier] Reject invalid type for DILocalVariable..

There's a type in a test, but otherwise, this LGTM.

Mon, Jan 7, 2:52 PM
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

Looks like these are unrelated failures. TestQueues started acting up last week because of a concurrency issue. I would recommend you attempt to re-land your commit again and see if any new failures show up. If you are unsure, feel free to ask here or on #llvm on IRC.

Mon, Jan 7, 1:09 PM
aprantl added inline comments to D55919: [DebugInfo] Omit location list entries with empty ranges.
Mon, Jan 7, 9:52 AM · debug-info
aprantl added a comment to D56084: Resubmit of rL345008 "Split MachinePipeliner code into header and cpp files".

@aprantl Found a workaround for the lldb-cmake bot: https://reviews.llvm.org/rL350346

I tested it locally with your patch reapplied and the error is fixed. Sorry again for the hassle. Please submit your patch. Thanks

@aprantl , @sgraenitz , I tried to resbumit the patch in https://reviews.llvm.org/rL350493 but unfortunately issues with the modules still appear: http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/314/steps/test/logs/stdio

it is now able to build but the lit test SymbolFile/PDB/class-layout.test is failing:

$ ":" "RUN: at line 4"
$ "E:\build_slave\lldb-x64-windows-ninja\build\bin\lldb-test.EXE" "symbols" "E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\SymbolFile\PDB\Output/ClassLayoutTest.cpp.exe"

  1. command stderr: error: Module has no symbol vendor.

error: command failed with exit status: 1
$ "E:\build_slave\lldb-x64-windows-ninja\build\bin\FileCheck.EXE" "E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\SymbolFile\PDB\class-layout.test"

  1. command stderr: E:\build_slave\lldb-x64-windows-ninja\llvm\tools\lldb\lit\SymbolFile\PDB\class-layout.test:15:8: error: CHECK: expected string not found in input CHECK: Module [[MOD:.*]] ^ <stdin>:1:1: note: scanning from here Module: E:\build_slave\lldb-x64-windows-ninja\build\tools\lldb\lit\SymbolFile\PDB\Output/ClassLayoutTest.cpp.exe

I have reverted the patch again for now as I am unable to reproduce the issue locally, your help will be much apprecicated

Mon, Jan 7, 9:50 AM
aprantl added inline comments to D56346: [dsymutil] Update unobfuscation logic..
Mon, Jan 7, 9:45 AM
aprantl added a comment to D56297: [DWARFUnit] Don't assume basic types..

Is the problem that it is a SubroutineType, but not a function *pointer*?
Generally, if the Verifier accepts it we ought to not crash, so this seems to be okay, but it might be worth asking whether this is really what you want your frontend to be creating since subroutinetypes don't have a size, etc.

I'm the author of the frontend that is generating this code. Thanks for the explanation of the problem. Based on this I tried similar code with clang:

Mon, Jan 7, 9:33 AM
aprantl added inline comments to D56337: [ADT] Remove attribute LLVM_ALWAYS_INLINE from several classes..
Mon, Jan 7, 9:29 AM
aprantl accepted D56337: [ADT] Remove attribute LLVM_ALWAYS_INLINE from several classes..
Mon, Jan 7, 9:27 AM

Fri, Jan 4

aprantl added inline comments to D56265: [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block.
Fri, Jan 4, 7:33 AM
aprantl added inline comments to D56151: [DebugInfo] PR40010: Avoid register coalesing altering DBG_VALUE valuations.
Fri, Jan 4, 7:30 AM

Thu, Jan 3

aprantl added a comment to D56297: [DWARFUnit] Don't assume basic types..

Is the problem that it is a SubroutineType, but not a function *pointer*?
Generally, if the Verifier accepts it we ought to not crash, so this seems to be okay, but it might be worth asking whether this is really what you want your frontend to be creating since subroutinetypes don't have a size, etc.

Thu, Jan 3, 3:42 PM
aprantl added inline comments to D56297: [DWARFUnit] Don't assume basic types..
Thu, Jan 3, 3:42 PM
aprantl added inline comments to D55859: noexternal 2/2: symbols.enable-external-lookup=false on all hosts (not just OSX).
Thu, Jan 3, 3:38 PM · Restricted Project
aprantl accepted D56027: [lldb] Fix ObjCExceptionRecognizedStackFrame to populate the list of recognized arguments.
Thu, Jan 3, 3:08 PM · Restricted Project
aprantl accepted D56115: [lldb] Check SafeToCallFunctions before calling functions in GetExceptionObjectForThread.

A testcase that triggers this situation would be nice, too.

Thu, Jan 3, 3:08 PM · Restricted Project
Herald added a reviewer for D56208: Add file-based synchronization to flaky test: serge-sans-paille.

I implemented a poor man's version of that in r350360.

Thu, Jan 3, 2:38 PM
aprantl added inline comments to D56265: [DebugInfo] MCP: collect and update DBG_VALUEs encountered in local block.
Thu, Jan 3, 12:53 PM

Wed, Jan 2

aprantl added a reviewer for D56218: Rearrange bitfield to allow for more space in file_idx.: friss.
Wed, Jan 2, 1:00 PM
aprantl created D56218: Rearrange bitfield to allow for more space in file_idx..
Wed, Jan 2, 1:00 PM
aprantl accepted D55987: [CodeGen] Skip over dbg-instr in twoaddr pass.
Wed, Jan 2, 11:05 AM
aprantl created D56208: Add file-based synchronization to flaky test.
Wed, Jan 2, 10:51 AM

Dec 21 2018

aprantl added inline comments to D55987: [CodeGen] Skip over dbg-instr in twoaddr pass.
Dec 21 2018, 2:09 PM

Dec 20 2018

aprantl accepted D55919: [DebugInfo] Omit location list entries with empty ranges.

Looks generally good to me.

Dec 20 2018, 9:00 AM · debug-info

Dec 19 2018

aprantl added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.

@aprantl Is the advantage of your suggested approach just that we don't have to define a new expression type? Obviously the interpretation is not the same as DW_OP_breg on other targets so as you say, either way there would have to be special logic in all the tools that consume it. Is this kind of repurposing of builtin primitives common?

Dec 19 2018, 10:01 AM · debug-info

Dec 18 2018

aprantl added a comment to D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing.

Sorry I forgot to comment: this should be fixed in r349533.

Dec 18 2018, 1:14 PM
aprantl added a comment to D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing.

Ah.. that's possible. REQUIRES needs to be very high up in the file, but I don't know what the exact threshold is

Dec 18 2018, 11:51 AM
aprantl accepted D55837: [cmake] Make libcxx(abi) a dependency when building in-tree clang for macOS.
Dec 18 2018, 11:50 AM · Restricted Project
aprantl added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.
I also found that the tools (e.g. LLVM or LLDB) are eager to know about number of registers (which we cannot determine) to allocate internal structures.
Dec 18 2018, 9:11 AM · debug-info
aprantl added a comment to D52634: [WebAssembly] Add DBG_VALUE with local operands location in WebAssemblyExplicitLocals pass.

Can you explain how this new operation is meant to work?

@aprantl, here is some description of what we are trying to achieve: https://github.com/WebAssembly/debugging/issues/1#issuecomment-448237641

In the nutshell, the WebAssembly extensions for the DWARF expression will allow us to express WebAssembly specific locations such as for locals, globals and operands stack. The consumers of this information will be limited to LLVM tools, web browser debuggers and AOT/JIT WebAssembly compilers.

Dec 18 2018, 8:40 AM · debug-info
aprantl added a comment to D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing.

Can you confirm that reverting this path actually fixes the issue? I'm asking because the only test that is executing this script has a REQUIRES: system-darwin line in it.

Dec 18 2018, 8:16 AM

Dec 17 2018

aprantl updated the diff for D55608: Make crashlog.py work or binaries with spaces in their names.

Got rid of the no_uuid regex and added even more testcases.

Dec 17 2018, 9:03 AM
aprantl added inline comments to D44100: [ASTImporter] Reorder fields after structure import is finished.
Dec 17 2018, 8:34 AM

Dec 15 2018

aprantl updated the diff for D55608: Make crashlog.py work or binaries with spaces in their names.

Fixed the reported problems with the regexes and added testcases with example lines. I used the funny test setup to avoid burdening the installed crashlog.py script with unit test code that would need to be parsed every time.

Dec 15 2018, 1:27 PM

Dec 14 2018

aprantl added inline comments to D55681: [llvm] API for encoding/decoding DWARF discriminators..
Dec 14 2018, 10:08 AM
aprantl added inline comments to D55681: [llvm] API for encoding/decoding DWARF discriminators..
Dec 14 2018, 8:18 AM

Dec 13 2018

aprantl added a comment to D55372: [DebugInfo] Emit "undef" DBG_VALUEs when SDNodes are optimised out.

Looks like this caused a noticeable shrinking of location lists, but that means we're getting more accurate: http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607042.4&highlight_run=117379 !

Dec 13 2018, 11:18 AM
aprantl added a comment to D51813: [Util] Refer to [s|z]exts of args when converting dbg.declares (fix PR35400).

Can you watch http://lnt.llvm.org/db_default/v4/nts/117379 (http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607042.4 and http://lnt.llvm.org/db_default/v4/nts/graph?plot.0=1357.1607043.4) after landing this?

Dec 13 2018, 11:14 AM · debug-info
aprantl added a comment to D55328: [CMake] Revised LLDB.framework builds.

Please be sure to watch all LLDB bots (green dragon and lab.llvm.org:8011) closely when landing this change.

Dec 13 2018, 11:03 AM
aprantl accepted D55328: [CMake] Revised LLDB.framework builds.

Let's give this a try.

Dec 13 2018, 11:02 AM
aprantl added inline comments to D55328: [CMake] Revised LLDB.framework builds.
Dec 13 2018, 11:02 AM
aprantl accepted D55332: [CMake] Python bindings generation polishing.
Dec 13 2018, 10:59 AM
aprantl added a comment to D55519: Reuse code from CGDebugInfo::getOrCreateFile() when creating the file for the DICompileUnit.

I believe I fixed this in r349065. (The DIFile used by the CU is special and distinct from the main source file. Its directory part specifies what becomes the DW_AT_comp_dir (the compilation directory), even if the source file was specified with an absolute path.)

Dec 13 2018, 9:57 AM · debug-info

Dec 12 2018

aprantl added inline comments to D55299: [LoopDeletion] Update debug values after loop deletion..
Dec 12 2018, 4:36 PM
aprantl added inline comments to D55626: [Reproducers] Add tests for functionality.
Dec 12 2018, 4:07 PM · Restricted Project
aprantl added inline comments to D55626: [Reproducers] Add tests for functionality.
Dec 12 2018, 4:07 PM · Restricted Project
aprantl updated the diff for D55608: Make crashlog.py work or binaries with spaces in their names.

Be more verbose about what is a valid arch.

Dec 12 2018, 12:05 PM
aprantl updated the diff for D55608: Make crashlog.py work or binaries with spaces in their names.
Dec 12 2018, 11:51 AM
aprantl created D55608: Make crashlog.py work or binaries with spaces in their names.
Dec 12 2018, 11:50 AM
aprantl created D55607: Make crashlog.py work when a .dSYM is present, but a binary is missing.
Dec 12 2018, 11:26 AM
aprantl accepted D55584: [LLDB] Simplify Boolean expressions.
Dec 12 2018, 9:25 AM · Restricted Project

Dec 11 2018

aprantl added a comment to D55584: [LLDB] Simplify Boolean expressions.

This one is probably less controversial than D55574 :-)

Dec 11 2018, 3:21 PM · Restricted Project
aprantl added inline comments to D55574: Remove else statements after returns.
Dec 11 2018, 3:21 PM
aprantl added a comment to D55574: Remove else statements after returns.

I'm mostly on board with making these changes, as it's good LLVM style to do this. I highlighted a couple changes that might warrant a closer look.

Dec 11 2018, 2:52 PM
aprantl abandoned D7016: Debug Info: Handle by-reference arguments correctly during inlining..

I just verified that this is no longer applicable as there is no longer a DW_OP_deref in the inlined function.

Dec 11 2018, 9:21 AM
aprantl closed D53677: Fix a bug PlatformDarwin::SDKSupportsModule.

Looks like this landed in r345274 and wasn't properly closed.

Dec 11 2018, 9:15 AM
aprantl added a comment to D55519: Reuse code from CGDebugInfo::getOrCreateFile() when creating the file for the DICompileUnit.

I also removed the other references to DIBuilder::createFile() in r348866.

Dec 11 2018, 9:03 AM · debug-info
aprantl abandoned D53961: GetNumChildren() version of D53530.
Dec 11 2018, 8:47 AM
aprantl created D55559: Remove the Disassembly benchmarks..
Dec 11 2018, 8:43 AM

Dec 10 2018

aprantl accepted D55396: [DebugInfo] Make sure CodeGenPrepare does not drop MD references to locals..

LGTM with outstanding comment addressed.

Dec 10 2018, 5:09 PM · debug-info
aprantl added inline comments to D55328: [CMake] Revised LLDB.framework builds.
Dec 10 2018, 10:24 AM
aprantl accepted D55513: [DeadArgElim] Fixes for dbg.values using dead arg/return values.

Thanks!

Dec 10 2018, 9:45 AM · debug-info
aprantl created D55519: Reuse code from CGDebugInfo::getOrCreateFile() when creating the file for the DICompileUnit.
Dec 10 2018, 9:44 AM · debug-info

Dec 7 2018

aprantl added a comment to D55085: Avoid emitting redundant or unusable directories in DIFile metadata entries.

Thanks, should be fixed in r348610!

Dec 7 2018, 9:08 AM · debug-info
aprantl accepted D55227: [DebugInfo] Don't drop dbg.value's of nullptr.

LGTM if @vsk's question is answered satisfactorily.

Dec 7 2018, 8:41 AM
aprantl accepted D55372: [DebugInfo] Emit "undef" DBG_VALUEs when SDNodes are optimised out.
Dec 7 2018, 8:38 AM