inglorion (Bob Haarman)
User

Projects

User does not belong to any projects.

User Details

User Since
Dec 15 2015, 10:55 AM (91 w, 6 d)

Recent Activity

Fri, Sep 15

inglorion committed rL313425: [docs] add Windows examples to ThinLTO.rst.
[docs] add Windows examples to ThinLTO.rst
Fri, Sep 15, 5:17 PM
inglorion closed D37943: [docs] add Windows examples to ThinLTO.rst by committing rL313425: [docs] add Windows examples to ThinLTO.rst.
Fri, Sep 15, 5:17 PM
inglorion created D37943: [docs] add Windows examples to ThinLTO.rst.
Fri, Sep 15, 4:58 PM

Mon, Sep 11

inglorion abandoned D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.
Mon, Sep 11, 3:28 PM
inglorion committed rL312965: [codeview] omit debug locations for nested exprs unless column info enabled.
[codeview] omit debug locations for nested exprs unless column info enabled
Mon, Sep 11, 3:14 PM
inglorion closed D37529: [codeview] omit debug locations for nested exprs unless column info enabled by committing rL312965: [codeview] omit debug locations for nested exprs unless column info enabled.
Mon, Sep 11, 3:14 PM
inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

renamed get{Nested,}ExpressionLocationsEnabled and moved it into CodeGetModule.cpp

Mon, Sep 11, 3:09 PM

Fri, Sep 8

inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

Of course, ApplyDebugLocation is also a perfectly legitimate way to add a debug location to nodes that are not nested inside nodes that already have a location. I updated the diff so that we do end up applying the location in such cases.

Fri, Sep 8, 5:19 PM
inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

added examples suggested by @zturner, verified step over and step into specific behavior matches MSVC, and added tests for them

Fri, Sep 8, 4:43 PM

Thu, Sep 7

inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

removed compound statement; that was only in there to double check that the debugger stops on the first child statement, but that's true with or without this change

Thu, Sep 7, 6:01 PM
inglorion added a comment to D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

@zturner: I am still thinking about your comment about other cases to test. My concern is that there are very many possible combinations.

Thu, Sep 7, 6:00 PM
inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

Following conversation with @rnk, I managed to whittle this down to a very small change that seems to do what we need. Step into specific works and single stepping through the program behaves similarly whether compiled with clang-cl or cl.

Thu, Sep 7, 5:29 PM

Wed, Sep 6

inglorion planned changes to D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

rnk and I talked about a different approach. The idea is to explicitly emit locations in some cases (e.g. inside compound statements, the braces of for loops, ...), and otherwise emit locations only when emitting column info or emitting non-codeview debug info. That may lead to more elegant code. I'll give it a try later.

Wed, Sep 6, 5:34 PM
inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

I limited the change to only calls, returns, and declarations. I also
updated the test case to include a multi-variable declaration, a while
loop, a for loop, and an if statement (after verifying the behavior in
the debugger, compared to MSVC). I discovered that there is a
difference between the generated info for DWARF with or without
-dwarf-column-info, so I included that in the test, too. I also made a
couple of minor changes that were suggested.

Wed, Sep 6, 4:38 PM
inglorion added inline comments to D37529: [codeview] omit debug locations for nested exprs unless column info enabled.
Wed, Sep 6, 3:20 PM
inglorion added inline comments to D37529: [codeview] omit debug locations for nested exprs unless column info enabled.
Wed, Sep 6, 3:09 PM
inglorion added inline comments to D37529: [codeview] omit debug locations for nested exprs unless column info enabled.
Wed, Sep 6, 2:34 PM
inglorion updated the diff for D37529: [codeview] omit debug locations for nested exprs unless column info enabled.

removed accidentally left in include and reformatted mangled comment

Wed, Sep 6, 1:21 PM
inglorion created D37529: [codeview] omit debug locations for nested exprs unless column info enabled.
Wed, Sep 6, 1:16 PM

Wed, Aug 30

inglorion accepted D37309: [codeview] Generalize DIExpression parsing to handle load chains.

lgtm modulo inline comment. Thanks!

Wed, Aug 30, 2:57 PM
inglorion added a comment to D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

Can we ship this?

Wed, Aug 30, 11:00 AM
inglorion committed rL312143: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional.
[codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional
Wed, Aug 30, 10:51 AM
inglorion closed D37279: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional by committing rL312143: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional.
Wed, Aug 30, 10:51 AM
inglorion updated the diff for D37279: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional.

Use None instead of its overly verbose equivalent.

Wed, Aug 30, 10:49 AM

Tue, Aug 29

inglorion updated the diff for D37279: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional.

rewritten using Optional instead of Expected

Tue, Aug 29, 6:08 PM
inglorion committed rL312055: [codeview] add missing break in CodeGen/AsmPrinter/DebugHandlerBase.cpp.
[codeview] add missing break in CodeGen/AsmPrinter/DebugHandlerBase.cpp
Tue, Aug 29, 3:55 PM
inglorion created D37279: [codeview] make DbgVariableLocation::extractFromMachineInstruction use Optional.
Tue, Aug 29, 3:02 PM
inglorion committed rL312035: [NFC] clang-format llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp.
[NFC] clang-format llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
Tue, Aug 29, 2:03 PM
inglorion committed rL312034: Reland r311957 [codeview] support more DW_OPs for more complete debug info.
Reland r311957 [codeview] support more DW_OPs for more complete debug info
Tue, Aug 29, 2:00 PM

Mon, Aug 28

inglorion committed rL311977: Revert "[codeview] support more DW_OPs for more complete debug info".
Revert "[codeview] support more DW_OPs for more complete debug info"
Mon, Aug 28, 9:09 PM
inglorion committed rL311976: Revert "[codeview] don't try to emit variable locations without registers".
Revert "[codeview] don't try to emit variable locations without registers"
Mon, Aug 28, 9:09 PM
inglorion added a comment to rL311969: [codeview] don't try to emit variable locations without registers.

I'm reverting this change and r311957 to stop the test failures.

Mon, Aug 28, 9:09 PM
inglorion added a comment to rL311969: [codeview] don't try to emit variable locations without registers.

The tests are flaky. Running ninja check-llvm multiple times will have them sometimes pass, sometimes fail.

Mon, Aug 28, 8:49 PM
inglorion added a comment to rL311969: [codeview] don't try to emit variable locations without registers.

According to http://lab.llvm.org:8011/builders/clang-x86_64-linux-selfhost-modules-2/builds/11192, this is causing tests to fail. I'm investigating.

Mon, Aug 28, 8:22 PM
inglorion committed rL311969: [codeview] don't try to emit variable locations without registers.
[codeview] don't try to emit variable locations without registers
Mon, Aug 28, 6:47 PM
inglorion added a comment to D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

Size numbers with this version:

Mon, Aug 28, 5:41 PM
inglorion updated the diff for D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

rebased

Mon, Aug 28, 5:36 PM
inglorion committed rL311957: [codeview] support more DW_OPs for more complete debug info.
[codeview] support more DW_OPs for more complete debug info
Mon, Aug 28, 5:08 PM
inglorion closed D36907: [codeview] support more DW_OPs for more complete debug info by committing rL311957: [codeview] support more DW_OPs for more complete debug info.
Mon, Aug 28, 5:08 PM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

changed 1-bit ints to unsigneds, ran clang-format

Mon, Aug 28, 4:44 PM
inglorion updated the diff for D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

I made the change a little more conservative. We now only use the single-record representation
when:

Mon, Aug 28, 4:30 PM
inglorion accepted D37233: [llvm-pdbutil] Print detailed S_UDT stats.

Mostly lgtm; please take a look at the couple of inline comments before landing.

Mon, Aug 28, 3:39 PM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

changes requested by @rnk

Mon, Aug 28, 3:01 PM
inglorion added inline comments to D36907: [codeview] support more DW_OPs for more complete debug info.
Mon, Aug 28, 2:35 PM

Fri, Aug 25

inglorion added a comment to D36907: [codeview] support more DW_OPs for more complete debug info.

Thanks for the feedback! I think I incorporated all of it. Does this look good now?

Fri, Aug 25, 4:13 PM

Thu, Aug 24

inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.
  • updated COFF/pieces.ll test to check for records we now emit
  • refactored to avoid iterating twice
Thu, Aug 24, 7:47 PM

Wed, Aug 23

inglorion updated the diff for D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

I still need to update/add tests.

Wed, Aug 23, 6:22 PM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

removed stray assert

Wed, Aug 23, 4:15 PM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.
  • rewrite non-deref ranges for variables we changed to references
  • factored out interpretation of debug info expressions
Wed, Aug 23, 4:10 PM

Tue, Aug 22

inglorion added a comment to D36907: [codeview] support more DW_OPs for more complete debug info.

@aprantl, I would like to do that, but this doesn't really correspond to any source anymore. I suppose you could take the LLVM IR part of it, take the offsets out of the DIExpressions, and then run it through llc -O0 -stop-after=prologepilog. Then you can edit the DIExpressions to be what you need them to be. Would it be useful to include that in a comment?

Tue, Aug 22, 4:51 PM
inglorion commandeered D37036: [codeview] use S_BPREL32, S_REGREL32, and S_REGISTER records when appropriate.

Thanks!

Tue, Aug 22, 4:38 PM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

How is this for the testcase? It checks positive offsets, negative offsets, and deref.

Tue, Aug 22, 4:05 PM
inglorion added inline comments to D36907: [codeview] support more DW_OPs for more complete debug info.
Tue, Aug 22, 11:48 AM
inglorion added a comment to D36907: [codeview] support more DW_OPs for more complete debug info.

Thanks for taking a look!

Tue, Aug 22, 10:23 AM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

arc diff --verbatim

Tue, Aug 22, 2:01 AM
inglorion updated the diff for D36907: [codeview] support more DW_OPs for more complete debug info.

Added support for DW_OP_deref, and a test case. This is now ready for review.

Tue, Aug 22, 2:00 AM

Aug 18 2017

inglorion added a comment to D36907: [codeview] support more DW_OPs for more complete debug info.

(Still a work in progress.)

Aug 18 2017, 5:29 PM
inglorion created D36907: [codeview] support more DW_OPs for more complete debug info.
Aug 18 2017, 5:29 PM

Jun 23 2017

inglorion added a comment to D33961: encode YAML scalars using double quotes so all values can be expressed.

@joerg, any comments?

Jun 23 2017, 2:20 PM

Jun 21 2017

inglorion committed rL305965: [codeview] respect signedness of APSInts when printing to YAML.
[codeview] respect signedness of APSInts when printing to YAML
Jun 21 2017, 3:32 PM
inglorion closed D34013: [codeview] respect signedness of APSInts when printing to YAML by committing rL305965: [codeview] respect signedness of APSInts when printing to YAML.
Jun 21 2017, 3:32 PM

Jun 20 2017

inglorion added a comment to D33961: encode YAML scalars using double quotes so all values can be expressed.

zturner: Do you have some time to look at this (D33961) and D34013?

Jun 20 2017, 2:52 PM

Jun 15 2017

inglorion committed rL305486: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects.
make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects
Jun 15 2017, 10:46 AM
inglorion closed D34065: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects by committing rL305486: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects.
Jun 15 2017, 10:46 AM

Jun 13 2017

inglorion added a comment to D34065: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects.

I see. Yes, it's unfortunate that this essentially repeats the list of projects in multiple places. On the other hand, I like having as much as possible in the part that can be modified without requiring a buildmaster reload, which would speak for leaving the checkout code in the builder. This is also what the sanitizer builders do. They trigger on changes to any of llvm, cfe, compiler-rt, libcxx, libcxxabi, libunwind, and lld, which we could also do here. It's basically triggering on everything, which is safe. Theoretically, it might increase resource usage a bit, but the ThinLTO builder as-is is basically always building, so the number of builds triggered and the amount of work the worker does should be the same. Would you like me to make that change?

Jun 13 2017, 3:50 PM

Jun 12 2017

inglorion added a comment to D34065: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects.

What do you mean by "the scheduling logic gets out of sync"?

Jun 12 2017, 3:33 PM

Jun 9 2017

inglorion updated the diff for D33961: encode YAML scalars using double quotes so all values can be expressed.

Implemented @zturner's suggestions

Jun 9 2017, 2:55 PM
inglorion created D34065: make AnnotatedBuilder use LLVMBuildFactory and depends_on_projects.
Jun 9 2017, 1:48 PM
inglorion accepted D34062: [llvm-pdbdump] Rename the tool to llvm-pdbutil.

Thanks! Assuming the tests pass, lgtm

Jun 9 2017, 11:34 AM
inglorion added inline comments to D33968: [codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection.
Jun 9 2017, 10:47 AM

Jun 8 2017

inglorion committed rL305043: [codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection.
[codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection
Jun 8 2017, 6:18 PM
inglorion closed D33968: [codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection by committing rL305043: [codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection.
Jun 8 2017, 6:18 PM
inglorion accepted D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.

Thanks! lgtm

Jun 8 2017, 4:03 PM
inglorion added inline comments to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.
Jun 8 2017, 3:09 PM
inglorion added inline comments to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.
Jun 8 2017, 2:44 PM
inglorion added inline comments to D34015: Allow subsections in raw output mode to be printed in the order they appear in the file..
Jun 8 2017, 2:31 PM
inglorion added a comment to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.

Hmm, good point. I hadn't considered that changing the name in the same change would mix a lot of mechanical changes with actual interesting to review changes. Let's keep them in two separate changes, then. Can you land them close together in time, though?

Jun 8 2017, 11:08 AM
inglorion added a comment to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.

Thanks for doing this, zturner! I have some more inline comments and questions.

Jun 8 2017, 11:06 AM
inglorion added a comment to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.

Oops, I submitted that before I was done reviewing. More comments probably coming soon.

Jun 8 2017, 10:24 AM
inglorion added a comment to D33996: Improve consistency and flexibility with llvm-pdbdump subcommands.

I would like to change the changes to these option names to coincide with renaming llvm-pdbdump to llvm-pdbtool (or some such) so that we only break the interface once.

Jun 8 2017, 10:23 AM

Jun 7 2017

inglorion added inline comments to D33961: encode YAML scalars using double quotes so all values can be expressed.
Jun 7 2017, 4:49 PM
inglorion created D34013: [codeview] respect signedness of APSInts when printing to YAML.
Jun 7 2017, 3:16 PM

Jun 6 2017

inglorion created D33968: [codeview] use 32-bit integer for RelocOffset in DebugLinesSubsection.
Jun 6 2017, 5:13 PM
inglorion added a comment to D33748: [PDB] Fix alignment of symbol record string.

This change is no longer necessary, right?

Jun 6 2017, 2:53 PM
inglorion added a comment to D33961: encode YAML scalars using double quotes so all values can be expressed.

The vast majority of changes here are updating tests to expect double quotes instead of single quotes. The actual change to the encoder is in llvm/lib/Support/YAMLTraits.cpp.

Jun 6 2017, 2:47 PM
inglorion created D33961: encode YAML scalars using double quotes so all values can be expressed.
Jun 6 2017, 2:47 PM

Jun 2 2017

inglorion accepted D33858: [PDB] Fix use after free..

lgtm. Thanks!

Jun 2 2017, 5:26 PM
inglorion accepted D33807: [CodeView] Allow Debug Subsections to be read/written in any order.

lgtm

Jun 2 2017, 12:29 PM

Jun 1 2017

inglorion requested changes to D33807: [CodeView] Allow Debug Subsections to be read/written in any order.

Happy you're doing this, but see inline comments for a few requested changes.

Jun 1 2017, 4:01 PM
inglorion abandoned D33612: [pdb] handle checksum and line info records that are not aligned to 4 bytes.

Superseded by D33785

Jun 1 2017, 3:10 PM
inglorion accepted D33785: [CodeView] Fix alignment / padding when writing symbol records.

lgtm modulo inline comment about messsage in the assert

Jun 1 2017, 2:38 PM
inglorion added a comment to D33785: [CodeView] Fix alignment / padding when writing symbol records.

I guess I'm saying it seems nice to always write aligned records until we come up with some reason to not write aligned records.

Jun 1 2017, 2:34 PM
inglorion added a comment to D33785: [CodeView] Fix alignment / padding when writing symbol records.

Thanks for fixing this, @zturner! I bumped into this a few days ago, but never really came up with a fix I was confident was the right thing.

Jun 1 2017, 11:30 AM

May 30 2017

inglorion committed rL304258: add generic annotated builder and clang-with-thin-lto-windows.
add generic annotated builder and clang-with-thin-lto-windows
May 30 2017, 5:11 PM
inglorion closed D33148: add generic annotated builder and clang-with-thin-lto-windows by committing rL304258: add generic annotated builder and clang-with-thin-lto-windows.
May 30 2017, 5:11 PM
inglorion updated the diff for D33148: add generic annotated builder and clang-with-thin-lto-windows.

pass -j argument to lit if jobs parameter is present

May 30 2017, 5:05 PM

May 26 2017

inglorion committed rL304047: [llvm-pdbdump] pdb2yaml: add an -all option to dump everything we can.
[llvm-pdbdump] pdb2yaml: add an -all option to dump everything we can
May 26 2017, 4:46 PM
inglorion closed D33613: [llvm-pdbdump] pdb2yaml: add an -all option to dump everything we can by committing rL304047: [llvm-pdbdump] pdb2yaml: add an -all option to dump everything we can.
May 26 2017, 4:46 PM
inglorion created D33613: [llvm-pdbdump] pdb2yaml: add an -all option to dump everything we can.
May 26 2017, 4:33 PM
inglorion added a comment to D33612: [pdb] handle checksum and line info records that are not aligned to 4 bytes.

This is not ready for review just yet. There are some other problems with the line information (all offsets show up as 0 when read with the DIA library) that I want to look into a bit more before I'm confident that this change is doing the right thing.

May 26 2017, 4:15 PM