aprantl (Adrian Prantl)
User

Projects

User does not belong to any projects.

User Details

User Since
Mar 2 2013, 8:12 AM (221 w, 8 h)

Recent Activity

Tue, May 23

aprantl accepted D32724: [Modules] Handle sanitizer feature mismatches when importing modules.

This is good from my point of view now. You might want to wait for an ok from @spyffe.

Tue, May 23, 2:56 PM

Fri, May 19

aprantl added a comment to D33370: Don't verify cross-imported bitcode in FunctionImporter.

It turns out that the SrcModule in FunctionImporter is in a really inconsistent intermediate state at the point where I ran the Verifier.

Oh right I forgot about this: lazy-loaded modules, even after materializing metadata, aren't passing the verified. This is super annoying.

After poking around a bit I came to the conclusion that it probably isn't even desirable to run the full-module verifier on each cross-imported module: Over the course of building the entire project the same module will be verified over and over again, which is a waste of resources. When we want to lazy-load metadata running a whole-module Verifier is also problematic.

There is something I'm missing: I thought the important part of the verifier was to be able to drop stalled debug information which is important for backward compatibility? This is the only reason I suggested doing it at this point of the process (before importing)...

Fri, May 19, 3:45 PM
aprantl created D33370: Don't verify cross-imported bitcode in FunctionImporter.
Fri, May 19, 3:17 PM
aprantl added a comment to D32368: LLVM C DIBuilder Creation APIs.

(Having unit tests will avoid

Fri, May 19, 1:34 PM
aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

Thanks for the review!

Fri, May 19, 10:40 AM
aprantl added a dependency for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator.: D33360: Rewrite llvm-lto's codegen() using ThinCodeGenerator::run(). NFC-ish..
Fri, May 19, 9:46 AM
aprantl added a dependent revision for D33360: Rewrite llvm-lto's codegen() using ThinCodeGenerator::run(). NFC-ish.: D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..
Fri, May 19, 9:46 AM
aprantl updated the diff for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

Depending on https://reviews.llvm.org/D33360, get rid of the verifier invocation in codegen()

Fri, May 19, 9:46 AM
aprantl created D33360: Rewrite llvm-lto's codegen() using ThinCodeGenerator::run(). NFC-ish..
Fri, May 19, 9:38 AM

Wed, May 17

aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

It is likely before run() was improved to be able to handle CodeGenOnly, we should retarget llvm-lto to use run() instead. I can do this if you want (likely tonight or tomorrow night)

Wed, May 17, 9:24 AM
aprantl accepted D33274: [DWARF] - Simplify RelocVisitor implementation..
Wed, May 17, 9:01 AM

Tue, May 16

aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

It seems to me that it indicates that the check in the codegen() method is redundant now. Why do we need it? What about just removing it?

Because llvm-lto invokes codegen() directly after loading the module itself (wihtout going through ThinLTOCodeGenerator for the loading):

It is likely before run() was improved to be able to handle CodeGenOnly, we should retarget llvm-lto to use run() instead. I can do this if you want (likely tonight or tomorrow night)

Tue, May 16, 4:16 PM
aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

It seems to me that it indicates that the check in the codegen() method is redundant now. Why do we need it? What about just removing it?

Tue, May 16, 3:25 PM
aprantl added inline comments to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..
Tue, May 16, 3:18 PM
aprantl updated the diff for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

Fix typos.

Tue, May 16, 3:16 PM
aprantl updated the diff for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

This assumes that there exists a main module, which is a foreign concept to ThinLTOCodegenerator till now AFAIK. I'm not sure what you're trying to express here?
Especially, this seems used only in the codegen, which is only use in a very specific mode (i.e. split model where you store optimized bitcode and reprocess to perform only the codegen). In such case either you want to verify all the modules that you're about to codegen or none. I have the impression that you'll check only the first one. Not sure if I missed something.

Tue, May 16, 3:15 PM
aprantl added inline comments to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..
Tue, May 16, 1:49 PM
aprantl updated the diff for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

What an entangled mess. This patch adds a boatload of testcases including the case where a function with broken debug info is cross-module-imported into a verified correct module. It also avoids re-verifying the module as much as possible.

Tue, May 16, 1:47 PM
aprantl accepted D33194: [DWARF] - Cleanup relocations proccessing..
Tue, May 16, 9:28 AM

Mon, May 15

aprantl updated the diff for D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

Yes I think you are right. There are too many places that assume the Verifier is as function pass.
The updated patch disabled input verification in the pass manager builder and manually verifies the input.

Mon, May 15, 4:07 PM
aprantl added a comment to D33194: [DWARF] - Cleanup relocations proccessing..

I'm assuming the checks are removed because they are already done elsewhere?

Mon, May 15, 9:44 AM
aprantl added inline comments to D33184: [DWARF] - Make collectAddressRanges() return section index in addition to Low/High PC.
Mon, May 15, 9:42 AM

Fri, May 12

aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

This turns out to be quite ugly.
There are two different code paths, by their llvm-lto options:
-thinlto-action=run uses PassManagerBuilder and sets VerifyInput=true, which causes a VerifierLegacyPass to be scheduled
-thinlto-action=codegen (which mimics llc) doesn't.
The obvious correct fix for -thinlto-action=codegen is to also schedule a Verifier pass. That is easy — but wait.

Fri, May 12, 8:20 PM
aprantl abandoned D31440: PR32382: Adapt to LLVM changes in DIExpression..
Fri, May 12, 8:05 PM
aprantl added inline comments to D33155: [DWARFv5] Support FORM_strp in the line table header.
Fri, May 12, 5:34 PM
aprantl added a comment to D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..

Interesting. The linker invocation I was debugging was using the

Fri, May 12, 2:21 PM
aprantl added a comment to D31136: [DWARF] - Speedup handling of relocations in DWARFContextInMemory..

I don't have an informed opinion at the moment, besides "let's benchmark it", so please go ahead for now. Once we switch over LLDB to use the LLVM DWARF parser I will likely revisit this in more depth.

Fri, May 12, 2:14 PM
aprantl added a comment to D33140: LTO: Don't verify modules twice in verifyMergedModuleOnce.

I don't think that is going to work, because of the report_fatal_error that also exits.

Fri, May 12, 2:09 PM
aprantl created D33151: ThinLTO: Verify bitcode before lauching the ThinLTOCodeGenerator..
Fri, May 12, 1:47 PM
aprantl created D33140: LTO: Don't verify modules twice in verifyMergedModuleOnce.
Fri, May 12, 10:29 AM

Tue, May 9

aprantl added inline comments to D32724: [Modules] Handle sanitizer feature mismatches when importing modules.
Tue, May 9, 4:51 PM
aprantl added inline comments to D29951: Load lazily the template specialization in multi-module setups..
Tue, May 9, 11:22 AM

Mon, May 8

aprantl accepted D32980: Use the frame index side table for byval and inalloca arguments.

This looks generally good (with outstanding comments addressed), thanks!

Mon, May 8, 3:08 PM
aprantl added inline comments to D32975: Make it illegal for two Functions to point to the same DISubprogram.
Mon, May 8, 1:45 PM
aprantl created D32975: Make it illegal for two Functions to point to the same DISubprogram.
Mon, May 8, 12:57 PM
aprantl added a comment to D32920: Don't add DBG_VALUE instructions for static allocas in dbg.declare.

Interesting, right!

Mon, May 8, 9:04 AM

Fri, May 5

aprantl accepted D32920: Don't add DBG_VALUE instructions for static allocas in dbg.declare.

This does not really have anything to do with the code in this patch though. I just really want to understand it.

Fri, May 5, 4:01 PM
aprantl added a comment to D32920: Don't add DBG_VALUE instructions for static allocas in dbg.declare.

I totally understand that it doesn't need to be tracked in the MMI table and in a DEBUG_VALUE, my question was that it sounded to my like you implied that the DWARF backend actively ignores DEBUG_VALUEs pointing to static allocas which I found really surprising (as there are legitimate reasons for allocas to be described by a DEBUG_VALUE) .

Fri, May 5, 3:55 PM
aprantl added a reviewer for D32724: [Modules] Handle sanitizer feature mismatches when importing modules: dexonsmith.
Fri, May 5, 3:45 PM
aprantl added a comment to D32724: [Modules] Handle sanitizer feature mismatches when importing modules.

Is it the right solution to use the module hash for correctness, or should the mismatch of the serialized langopts trigger a module rebuild and the module hash is only there to tune the performance/disk size tradeoff?

Fri, May 5, 3:45 PM
aprantl added a comment to D32920: Don't add DBG_VALUE instructions for static allocas in dbg.declare.

An llvm.dbg.declare of a static alloca is always added to the
MachineFunction dbg variable map, so these values are entirely
redundant.

Fri, May 5, 2:33 PM
aprantl resigned from D32821: Add DWARF verifiers to verify address ranges are correct and scoped correctly..

David, are all your concerns addressed?
Otherwise from my end this change LGTM (with all outstanding issues addressed).

Fri, May 5, 9:31 AM
aprantl added inline comments to D32821: Add DWARF verifiers to verify address ranges are correct and scoped correctly..
Fri, May 5, 9:24 AM
aprantl added inline comments to D32865: [llvm-dwarfdump] - Print an error message if section decompression failed..
Fri, May 5, 8:29 AM

Thu, May 4

aprantl added a comment to D32618: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section).

By the way: you probably noticed that since last week have a brand new DWARF Verifier (accessible via dwarfdump --verify). It might be nice to develop Verifier checks alongside with the support for new sections/formats.

Thu, May 4, 1:40 PM
aprantl accepted D32865: [llvm-dwarfdump] - Print an error message if section decompression failed..

Seems generally fine, couple of suggestions inline.

Thu, May 4, 8:23 AM

Wed, May 3

aprantl accepted D32812: Break verification down into smaller functions to keep code clean..
Wed, May 3, 11:29 AM
aprantl accepted D32809: Create DWARFVerifier.cpp and .h and move all DWARF verification code over into it..

Thanks!

Wed, May 3, 9:14 AM
aprantl added a comment to D32745: Correct debug info bit offset calculation for big-endian targets.

Ah. Thanks for clarifying! We should remove it, then.

Wed, May 3, 8:59 AM
aprantl added a comment to D32777: Remap metadata attached to global variables..

I'm not sure if globals are still reachable via !dbg, but it certainly used to be possible. Adrian, do you know the current state?

I'm not sure I understand your question:

Wed, May 3, 8:58 AM
aprantl accepted D32771: Verify that no compile units share the same line table in "llvm-dwarfdump --verify".

Sure. I'm fine with a separate commit that breaks the Verifier out into its own file and also breaks the current HandleDebugLine function (which btw. should start with a lower-case character) into smaller functions.

Wed, May 3, 8:38 AM

Tue, May 2

aprantl requested changes to D32771: Verify that no compile units share the same line table in "llvm-dwarfdump --verify".
Tue, May 2, 5:06 PM
aprantl added a comment to D32713: [DWARFv5] Parse new line-table header format..

Thanks! I'll leave this to David to mark as accepted.

Tue, May 2, 12:50 PM
aprantl accepted D32722: Verify that all references point to actual DIEs in "llvm-dwarfdump --verify".
Tue, May 2, 12:48 PM
aprantl accepted D32745: Correct debug info bit offset calculation for big-endian targets.

Doesn't the test need some kind of REQUIRES: mips line?
Otherwise looks fine.

Tue, May 2, 9:10 AM

Mon, May 1

aprantl accepted D32728: Make DWARFDebugLine use StringRef for dir/file tables. NFC.

I think this looks much nicer!

Mon, May 1, 6:21 PM
aprantl added inline comments to D32707: lldb-dwarfdump -verify [-quiet].
Mon, May 1, 3:01 PM
aprantl accepted D32707: lldb-dwarfdump -verify [-quiet].

LGTM once outstanding issues are addressed.

Mon, May 1, 2:58 PM
aprantl added inline comments to D32707: lldb-dwarfdump -verify [-quiet].
Mon, May 1, 2:40 PM
aprantl added inline comments to D32707: lldb-dwarfdump -verify [-quiet].
Mon, May 1, 2:14 PM
aprantl added a comment to D32713: [DWARFv5] Parse new line-table header format..

seems generally fine, couple of stylistic nits inline.

Mon, May 1, 12:48 PM
aprantl added inline comments to D32618: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section).
Mon, May 1, 11:45 AM

Fri, Apr 28

aprantl updated the diff for D32648: Remove line and file from DINamespace.

Address further review feedback from David.

Fri, Apr 28, 3:12 PM
aprantl updated the diff for D32648: Remove line and file from DINamespace.

Don't make anonymous namespaces distinct.

Fri, Apr 28, 2:23 PM
aprantl added a comment to D32625: lldb-dwarfdump -verify [-quiet].

First off, thank you very much for getting this started!

Fri, Apr 28, 1:10 PM
aprantl updated the diff for D32648: Remove line and file from DINamespace.

Added a negative check for decl_line/decl_file to DebugInfo/Generic/namespace.ll

Fri, Apr 28, 11:52 AM
aprantl added a comment to D32648: Remove line and file from DINamespace.

I'm all for this, couple of questions though:

  • When would the use of "distinct" on an anonymous namespace matter? Since LLVM uses cross-CU references to refer to entities in different CUs, never importing/moving entities between CUs, the anonymous namespaces shouldn't collide, should they? (anymore than named namespaces would collide when there are different entities in different CUs but both in the same named namespace - LLVM still produces two separate namespaces, one in each CU, one with each of the respective entities)?
Fri, Apr 28, 11:17 AM
aprantl updated the diff for D32648: Remove line and file from DINamespace.

Add a bitcode upgrade for anonymous namespaces.
Thanks, Duncan!

Fri, Apr 28, 10:21 AM
aprantl created D32648: Remove line and file from DINamespace.
Fri, Apr 28, 9:42 AM

Apr 27 2017

aprantl added a comment to D32618: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section).

Also, apologies for not handling Mach-O.

Typically we try to do a best effort on all supported containers (ELF, Mach-O, COFF) when adding new features.
Could you outline what is necessary there? Is it just coming up with a short-enough section name or is there more to it?

That is probably it. Basically, the hardest part is just making string references unique to the unit (compile unit, type unit) and putting all the relocatable string references into the new section. Nothing that should be specific to the object file format. I can take a stab at it, it shouldn't take too long.

Apr 27 2017, 4:59 PM
aprantl added a comment to D32618: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section).

Also, apologies for not handling Mach-O.

Typically we try to do a best effort on all supported containers (ELF, Mach-O, COFF) when adding new features.
Could you outline what is necessary there? Is it just coming up with a short-enough section name or is there more to it?

Apr 27 2017, 4:33 PM
aprantl added a comment to D32618: DWARF: Implementation of v5 string offsets table (.debug_str_offsets[.dwo] section).

Thanks! The patch may be large, but the changes are mostly straightforward and generally look good to me. I found a couple of nits inline.

Apr 27 2017, 3:59 PM
aprantl accepted D31440: PR32382: Adapt to LLVM changes in DIExpression..

I think I may haved screwed up the phabricator process for this one.
This landed as r300523.

Apr 27 2017, 1:50 PM
aprantl added a comment to D32603: Build the Apple-style stage2 with modules and full debug info.

It would simplify fallout, reverting, triaging issues, understanding compile time impact, etc., if the debug info change was left for a separate commit (maybe waiting a day). Thoughts?

Apr 27 2017, 11:26 AM
aprantl updated the diff for D32603: Build the Apple-style stage2 with modules and full debug info.

Address review feedback.

Apr 27 2017, 11:21 AM
aprantl added a comment to D32603: Build the Apple-style stage2 with modules and full debug info.

@beanz: Would sorting all these set() commands alphabetically break anything?

Apr 27 2017, 11:17 AM
aprantl created D32603: Build the Apple-style stage2 with modules and full debug info.
Apr 27 2017, 11:17 AM
aprantl accepted D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..

Thanks!

Apr 27 2017, 9:01 AM

Apr 26 2017

aprantl closed D32560: Turn DISubprogram into a variable-length node.

r301498

Apr 26 2017, 4:58 PM
aprantl updated the diff for D32559: Debug info: Add support for DW_TAG_thrown_type..

Expand testcase to use more than one thrown type.
Wire up DIBuilder support & add a unittest.

Apr 26 2017, 3:27 PM
aprantl added a dependent revision for D32559: Debug info: Add support for DW_TAG_thrown_type.: D32560: Turn DISubprogram into a variable-length node.
Apr 26 2017, 2:30 PM
aprantl added a dependency for D32560: Turn DISubprogram into a variable-length node: D32559: Debug info: Add support for DW_TAG_thrown_type..
Apr 26 2017, 2:30 PM
aprantl created D32560: Turn DISubprogram into a variable-length node.
Apr 26 2017, 2:30 PM
aprantl created D32559: Debug info: Add support for DW_TAG_thrown_type..
Apr 26 2017, 2:30 PM
aprantl added a comment to D32509: Replace HashString algorithm with xxHash64.

@echristo: Is there anything besides LLVM tests that depends on llvm-dwarfdump's output ordering for the .gnu_pubnames section?

Apr 26 2017, 8:50 AM
aprantl added inline comments to D32509: Replace HashString algorithm with xxHash64.
Apr 26 2017, 8:48 AM
aprantl added a reviewer for D32509: Replace HashString algorithm with xxHash64: echristo.
Apr 26 2017, 8:48 AM

Apr 25 2017

aprantl added inline comments to D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..
Apr 25 2017, 2:55 PM
aprantl added inline comments to D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..
Apr 25 2017, 9:14 AM

Apr 21 2017

aprantl added a comment to D32368: LLVM C DIBuilder Creation APIs.

Could you also add some unit tests for the new interface?

Apr 21 2017, 1:37 PM
aprantl added inline comments to D32368: LLVM C DIBuilder Creation APIs.
Apr 21 2017, 1:30 PM
aprantl added inline comments to D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..
Apr 21 2017, 8:53 AM

Apr 20 2017

aprantl added inline comments to D32315: Introduce a new DWARFContext::getInliningInfoForAddress API to expose pointers to strings stored in DWARF file..
Apr 20 2017, 3:38 PM
aprantl added inline comments to D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..
Apr 20 2017, 2:02 PM
aprantl accepted D30785: [DWARF] Versioning for DWARF constants; verify FORMs.

ok.

Apr 20 2017, 10:21 AM
aprantl added a comment to D30785: [DWARF] Versioning for DWARF constants; verify FORMs.

Besides my above comment, I'm very much looking forward to this commit. It will also help a lot with implementing dwarfdump -verify functionality.

Apr 20 2017, 9:49 AM
aprantl added inline comments to D31604: [DebugInfo][X86] Improve X86 Optimize LEAs handling of debug values..
Apr 20 2017, 9:41 AM
aprantl added a comment to D32284: [DWARF] - Refactoring: localize handling of relocations in a single place..

LGTM with inline feedback addressed. Thanks!

Apr 20 2017, 9:04 AM

Apr 19 2017

aprantl added a comment to D31755: [DebugInfo][X86] Fix handling of DBG_VALUE's in post-RA scheduler..

Rather than scanning forward for DBG_VALUEs, is there a more systematic way to find them? E.g., by iterating over USEs? If not, doing it this way is probably fine.

Apr 19 2017, 3:53 PM
aprantl updated the diff for D32246: Don't emit CFI instructions at the end of a function.

Address review feedback. Thanks!

Apr 19 2017, 3:36 PM