aprantl (Adrian Prantl)
User

Projects

User Details

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

Recent Activity

Fri, Nov 17

aprantl added a comment to D40199: llvm-dwarfdump --verify is incorrectly saying all DW_AT_location attributes with locations lists are invalid..

Do you happen to have a minimal reproducer that we could check in as an assembler testcase?

Fri, Nov 17, 3:36 PM · debug-info

Wed, Nov 15

aprantl accepted D40104: [SelectionDAG] Consolidate (t|T)ransferDbgValues methods, NFC..

good catch!

Wed, Nov 15, 5:38 PM · debug-info

Tue, Nov 14

aprantl added a reviewer for D40057: Simplify file handling in dsymutil: friss.
Tue, Nov 14, 5:15 PM
aprantl added inline comments to D39982: [IRBuilder] Set the insert point and debug location together.
Tue, Nov 14, 1:50 PM · debug-info
aprantl accepted D40042: Make salvageDebugInfo of casts work for dbg.declare and dbg.addr.

That looks even better!

Tue, Nov 14, 1:20 PM
aprantl added inline comments to D39985: [SCEV] Apply a single debug loc when expanding a SCEV.
Tue, Nov 14, 11:25 AM
aprantl added inline comments to D39986: [LSR] Expand: Use the replaced value's debug loc (PR25630).
Tue, Nov 14, 11:23 AM
aprantl accepted D40042: Make salvageDebugInfo of casts work for dbg.declare and dbg.addr.

Nice!

Tue, Nov 14, 11:21 AM
aprantl accepted D39981: [DebugInfo] Fix potential CU mismatch for SubprogramScopeDIEs..
Tue, Nov 14, 10:24 AM
aprantl added inline comments to D39985: [SCEV] Apply a single debug loc when expanding a SCEV.
Tue, Nov 14, 7:06 AM
aprantl accepted D39986: [LSR] Expand: Use the replaced value's debug loc (PR25630).
Tue, Nov 14, 7:03 AM

Mon, Nov 13

aprantl added a comment to D39982: [IRBuilder] Set the insert point and debug location together.

Thanks, this is looking really good!

Mon, Nov 13, 7:10 PM · debug-info
aprantl added inline comments to D39982: [IRBuilder] Set the insert point and debug location together.
Mon, Nov 13, 7:06 PM · debug-info
aprantl added a reviewer for D39982: [IRBuilder] Set the insert point and debug location together: whitequark.
Mon, Nov 13, 7:00 PM · debug-info
aprantl added inline comments to D39985: [SCEV] Apply a single debug loc when expanding a SCEV.
Mon, Nov 13, 6:57 PM
aprantl added inline comments to D39986: [LSR] Expand: Use the replaced value's debug loc (PR25630).
Mon, Nov 13, 6:53 PM
aprantl added inline comments to D39985: [SCEV] Apply a single debug loc when expanding a SCEV.
Mon, Nov 13, 6:50 PM
aprantl added reviewers for D39985: [SCEV] Apply a single debug loc when expanding a SCEV: dblaikie, probinson.
Mon, Nov 13, 6:47 PM
aprantl added inline comments to D39981: [DebugInfo] Fix potential CU mismatch for SubprogramScopeDIEs..
Mon, Nov 13, 3:52 PM
aprantl abandoned D4832: Debug info: Teach the SDag type legalizer how to split up debug info for integer values that are split into a hi and lo part. .
Mon, Nov 13, 1:28 PM

Fri, Nov 10

aprantl added a reviewer for D39925: [MC] Expose hasDefOfPhysReg(..) in the public MCInstrDesc interface.: MatzeB.
Fri, Nov 10, 2:55 PM

Thu, Nov 9

aprantl accepted D38024: [RegisterCoalescer] Move debug value after rematerialize trivial def.

seems plausible, thanks!

Thu, Nov 9, 11:33 AM
aprantl added a comment to D38088: Fix out-of-order stepping behavior in programs with hoisted constants..

Paul, can you commit this?

Thu, Nov 9, 11:30 AM
aprantl accepted D38088: Fix out-of-order stepping behavior in programs with hoisted constants..
Thu, Nov 9, 11:29 AM

Wed, Nov 8

aprantl added a comment to D39622: Fix type debug information generation for enum-based template specialization.

For clarification: what is the "symbols table" you are referring to in the description?

Wed, Nov 8, 10:38 AM · Restricted Project
aprantl accepted D39503: Let replaceVTableHolder accept any type.
Wed, Nov 8, 9:54 AM

Tue, Nov 7

aprantl accepted D33155: [DWARFv5] Support FORM_strp in the line table header.

Thanks for converting the other testcase.

Tue, Nov 7, 10:40 AM

Mon, Nov 6

aprantl added a comment to D39669: DAG: Preserve nuw when reassociating adds.

If it does break a subsequent commit, could you commit that first, and then apply this to fix the regressions introduced by the other commit?

Mon, Nov 6, 11:22 AM
aprantl accepted D39628: [DebugInfo] Unify logic to merge DILocations. NFC..

Oh.. I overlooked the early exit. Yeah I guess we can do this.

Mon, Nov 6, 11:20 AM
aprantl added a comment to D39628: [DebugInfo] Unify logic to merge DILocations. NFC..

What about the common-parent-scope-finding logic?

Mon, Nov 6, 11:05 AM

Sun, Nov 5

aprantl accepted D39633: Remove \brief from doxygen comments in PrettyPrinter.h.

Thanks!

Sun, Nov 5, 8:03 AM · Restricted Project

Sat, Nov 4

aprantl added inline comments to D39622: Fix type debug information generation for enum-based template specialization.
Sat, Nov 4, 10:07 AM · Restricted Project
aprantl added a comment to D39628: [DebugInfo] Unify logic to merge DILocations. NFC..

It looks like this will cause DILocation::getMergedLocation() to create new DILocations? When this happens at the MIR level this will cause problems with the textual MIR output because the new locations aren't printed. Should we introduce a flag to disable creation of new metadata?

Sat, Nov 4, 8:20 AM

Fri, Nov 3

aprantl added inline comments to D39622: Fix type debug information generation for enum-based template specialization.
Fri, Nov 3, 4:22 PM · Restricted Project
aprantl added inline comments to D39622: Fix type debug information generation for enum-based template specialization.
Fri, Nov 3, 4:22 PM · Restricted Project
aprantl added a comment to D39622: Fix type debug information generation for enum-based template specialization.

Can you add a testcase?

Fri, Nov 3, 4:20 PM · Restricted Project
aprantl added reviewers for D39622: Fix type debug information generation for enum-based template specialization: aprantl, dblaikie.
Fri, Nov 3, 4:19 PM · Restricted Project
aprantl added a comment to D39605: [debuginfo-tests] Decouple debuginfo-tests from the clang source tree..

I'm adding Chris Matthews and Mike Edwards who are maintaining zorg and the jenkins configuration on green dragon to the review, so we can coordinate this.

Fri, Nov 3, 2:19 PM
aprantl added reviewers for D39605: [debuginfo-tests] Decouple debuginfo-tests from the clang source tree.: sqlbyme, cmatthews.
Fri, Nov 3, 2:17 PM
aprantl requested changes to D39605: [debuginfo-tests] Decouple debuginfo-tests from the clang source tree..

This change is generally fine, but since this will break the workflow that the green-dragon bots are using where the repository is checked out into llvm/tools/clang/test, we must update the bots at the same time. I think this involves both changes to zorg and the jenkins configuration. I'm marking the review as "request changes" to reflect that.

Fri, Nov 3, 2:17 PM

Thu, Nov 2

aprantl updated the diff for D39345: SCEV: preserve debug information attached to PHI nodes..

Remove the lambda; it now looks silly.

Thu, Nov 2, 3:57 PM
aprantl updated the diff for D39345: SCEV: preserve debug information attached to PHI nodes..

Simplify the patch because the SCEV for old and new phi are always the same.

Thu, Nov 2, 3:54 PM
aprantl added a comment to D39568: Clean up DIBuilder.h comments.

thanks! You never know what kind of tools try to parse C header files.

Thu, Nov 2, 2:17 PM
aprantl added inline comments to D39568: Clean up DIBuilder.h comments.
Thu, Nov 2, 2:16 PM
aprantl added inline comments to D39561: [dsymutil][doc] Improve wording and rename file..
Thu, Nov 2, 11:31 AM
aprantl accepted D39561: [dsymutil][doc] Improve wording and rename file..

LGTM, but could you also incorporate my feedback from https://reviews.llvm.org/rL317221?

Thu, Nov 2, 11:30 AM
aprantl added inline comments to rL317221: [dsymutil] Add a manpage for dsymutil.
Thu, Nov 2, 11:01 AM
aprantl added inline comments to D39503: Let replaceVTableHolder accept any type.
Thu, Nov 2, 9:31 AM
aprantl added a comment to D39345: SCEV: preserve debug information attached to PHI nodes..

Thanks for confirming! Also, unfortunate :-(
Do you have any feedback on the patch for the zero-delta-case as is that I should be addressing before landing this?

Thu, Nov 2, 8:56 AM

Wed, Nov 1

aprantl added inline comments to D39503: Let replaceVTableHolder accept any type.
Wed, Nov 1, 2:02 PM
aprantl accepted D39496: [dsymutil][NFC} Rename thread related command line options.
Wed, Nov 1, 10:01 AM
aprantl accepted D39494: [SimplifyCFG] Discard speculated dbg intrinsics.

This is an obviously safe/correct thing to do. Thanks!

Wed, Nov 1, 9:01 AM
aprantl added a comment to D39355: [dsymutil] Implement the --threads option.

How does the conflict manifest? The option registry shouldn't conflict because dsymutil's option is in a separate category. Or is it the alias that is conflicting? It would be good to understand this. Renaming the option would break compatibility for existing users of dsymutil, so we should avoid that.

Wed, Nov 1, 8:50 AM

Tue, Oct 31

aprantl added a comment to D39355: [dsymutil] Implement the --threads option.

Our build uses -DLLVM_BUILD_LLVM_DYLIB=ON and -DLLVM_LINK_LLVM_DYLIB=ON, and we include lld, which my theory is then that both options are statically included in libLLVM, even though they should otherwise share no code.

Tue, Oct 31, 4:19 PM
aprantl updated the diff for D39345: SCEV: preserve debug information attached to PHI nodes..

Meanwhile I uploaded the simplified patch that just handles the common case in indvars.

Tue, Oct 31, 11:27 AM
aprantl added a comment to D39345: SCEV: preserve debug information attached to PHI nodes..

I moved the logic over to indvars now, and that seems to fit naturally into WidenIV::createWideIV() with about the same functionality as above.

Tue, Oct 31, 11:12 AM

Mon, Oct 30

aprantl added a comment to D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk..

I see. Then it makes sense to do it this way. Thanks for clarifying!

Mon, Oct 30, 7:55 PM
aprantl accepted D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk..

This works for me, but as I said previously, perhaps we can get by with just not having any variables described in the thunk to further simplify the code.

Mon, Oct 30, 5:29 PM
aprantl added a comment to D33155: [DWARFv5] Support FORM_strp in the line table header.

Thanks, that sounds great! As far as I know there are only very few bots that don't build the X86 backend at the moment so I wouldn't be too concerned about that.

Mon, Oct 30, 2:42 PM
aprantl added a comment to D39345: SCEV: preserve debug information attached to PHI nodes..

First, should this logic be here or should it be in IndVarSimplify? I ask because there are transformations that can introduce new AddRecs into an existing loop without replacing the induction variable. Only the transformation using the SCEVExpander knows if it is planning to actually replace the induction variable.

Mon, Oct 30, 9:10 AM
aprantl added inline comments to D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk..
Mon, Oct 30, 8:57 AM
aprantl added a comment to D39396: Fix for PR33930. Short-circuit metadata mapping when cloning a varargs thunk..

The only drawback is that the cloned function's (the thunk's) DILocalVariableNodes will not appear in the newly cloned DISubprogram's node variable list. Instead, the new node points to the original function's variable list. However, since the only difference between the original function and the thunk is the this-ptr adjustment, the resulting debug information is correct, at least in the test cases I have looked at.

Does the thunk contain any local variables other than function arguments?
What I'm getting at is: If this is only a thunk, do we need any local variables at all?

Mon, Oct 30, 8:56 AM

Fri, Oct 27

aprantl added inline comments to D33155: [DWARFv5] Support FORM_strp in the line table header.
Fri, Oct 27, 3:35 PM
aprantl updated the diff for D39345: SCEV: preserve debug information attached to PHI nodes..

Here is a slightly more general version that includes most of Hal's feedback. I will create a couple of more interesting testcases next to see how to handle cases where the result of subtracting the two recurrences is not a constant.

Fri, Oct 27, 11:47 AM
aprantl accepted D39355: [dsymutil] Implement the --threads option.
Fri, Oct 27, 11:35 AM
aprantl added inline comments to D33155: [DWARFv5] Support FORM_strp in the line table header.
Fri, Oct 27, 10:49 AM
aprantl added inline comments to D39345: SCEV: preserve debug information attached to PHI nodes..
Fri, Oct 27, 10:42 AM
aprantl added reviewers for D39345: SCEV: preserve debug information attached to PHI nodes.: aschwaighofer, atrick.
Fri, Oct 27, 9:19 AM
aprantl added inline comments to D39355: [dsymutil] Implement the --threads option.
Fri, Oct 27, 8:51 AM

Thu, Oct 26

aprantl added inline comments to D39345: SCEV: preserve debug information attached to PHI nodes..
Thu, Oct 26, 5:55 PM
aprantl added a reviewer for D39345: SCEV: preserve debug information attached to PHI nodes.: hfinkel.
Thu, Oct 26, 4:13 PM
aprantl planned changes to D39345: SCEV: preserve debug information attached to PHI nodes..
Thu, Oct 26, 4:13 PM
aprantl created D39345: SCEV: preserve debug information attached to PHI nodes..
Thu, Oct 26, 2:55 PM
aprantl accepted D39343: Do not add discriminator encoding for debug intrinsics..

Correct, discriminators don't make sense for debug intrinsics.

Thu, Oct 26, 2:07 PM
aprantl accepted D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block.
Thu, Oct 26, 1:13 PM
aprantl added inline comments to D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block.
Thu, Oct 26, 11:15 AM
aprantl added inline comments to D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block.
Thu, Oct 26, 11:14 AM
aprantl added a comment to D39305: Simplify codegen and debug info generation for block context parameters..

The original code doesn't make any sense; it looks like the indirection is backwards. We just built a debug variable corresponding to the block descriptor pointer parameter, so LocalAddr should be dbg.declare'd to be that variable and Arg should be dbg.value'd. It looks like the real fix in your patch is just getting that right rather than anything to do with the alloca.

Thu, Oct 26, 11:04 AM
aprantl accepted D39336: [dsymutil] Check AttrInfo.Name validity before using it.
Thu, Oct 26, 10:18 AM
aprantl added a comment to D39294: [llvm-dwarfdump] - Teach verifier to report broken DWARF expressions..

Thanks!

Thu, Oct 26, 9:09 AM
aprantl added a comment to D39310: [CGBlocks] Improve line info in backtraces containing *_helper_block.

Thanks! This looks generally good, I have one pending question inline.

Thu, Oct 26, 9:03 AM

Wed, Oct 25

aprantl created D39305: Simplify codegen and debug info generation for block context parameters..
Wed, Oct 25, 2:24 PM
aprantl accepted D39294: [llvm-dwarfdump] - Teach verifier to report broken DWARF expressions..

Thanks!

Wed, Oct 25, 1:51 PM

Tue, Oct 24

aprantl added a comment to D39185: [llvm-dwarfdump] - Fix array out of bounds access crash..

Out of curiosity: does llvm-dwarfdump --verify fail on your testcase?

Yes, it complains:

Awesome!

Tue, Oct 24, 8:58 AM
aprantl added a comment to D39119: [llvm-dwarfdump] - Teach tool about few GNU call_sites constants..

Could you also teach the DWARFVerifier about these attributes?

I committed cleanup for testcase (rL316428). Now it passes -verify without errors.

And tried to investigate what can be needed for DWARFVerifier.
This attributes has following forms:

  1. DW_AT_GNU_all_call_sites, DW_FORM_flag_present
  2. DW_AT_GNU_call_site_value, DW_FORM_exprloc

    First one seems does not need anything. DW_FORM_exprloc seems need some verification, but DW_AT_GNU_call_site_value is already proccessed here (implemented in current patch): https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFDie.cpp#L238 together with another location expressions. And errors are reported: https://github.com/llvm-mirror/llvm/blob/master/lib/DebugInfo/DWARF/DWARFExpression.cpp#L259.

    Though that happens with -v and not with -verify for some reason. So if I break the expression, with verbose option I get: ` DW_AT_GNU_call_site_value [DW_FORM_exprloc] (DW_OP_lit4, decoding error. 00 00 00 00 00) ` But with -verify it reports no any errors. Is it correct ? I would expect verify to report failture. Its unrelative to my changes though.

    Aside from this issue with DW_FORM_exprloc above, It seems nothing else should be done for these 2 GNU all_call_sites attributes on DWARFVerifier side, am I right ? Should I check something else here ?
Tue, Oct 24, 8:49 AM
aprantl accepted D39185: [llvm-dwarfdump] - Fix array out of bounds access crash..

Out of curiosity: does llvm-dwarfdump --verify fail on your testcase?

Tue, Oct 24, 8:47 AM

Mon, Oct 23

aprantl added inline comments to D39207: [llvm-objcopy] Add support for dwarf fission.
Mon, Oct 23, 3:34 PM
aprantl added inline comments to D39185: [llvm-dwarfdump] - Fix array out of bounds access crash..
Mon, Oct 23, 9:24 AM

Oct 20 2017

aprantl accepted D39119: [llvm-dwarfdump] - Teach tool about few GNU call_sites constants..

Could you also teach the DWARFVerifier about these attributes?
You might also want to add support for the standardized DWARF 5 version of DW_AT_call_site & friends.

Oct 20 2017, 10:00 AM

Oct 13 2017

aprantl added a reviewer for D38879: [llvm-dwarfdump] - Teach tool to parse DW_CFA_GNU_args_size.: JDevlieghere.
Oct 13 2017, 7:31 AM
aprantl added a comment to D38879: [llvm-dwarfdump] - Teach tool to parse DW_CFA_GNU_args_size..

Currently llvm-dwarfdump runs into llvm_unreachable when faces DW_CFA_GNU_args_size.

Oct 13 2017, 7:31 AM
aprantl accepted D38879: [llvm-dwarfdump] - Teach tool to parse DW_CFA_GNU_args_size..

Do you need to update the Verifier, too? Does llvm-dwarfdump -verify work on this input?

Oct 13 2017, 7:28 AM

Oct 12 2017

aprantl accepted D38830: [DWARF] Fix bad comparator in sortGlobalExprs..

LGTM together with Mikael's testcase modified to CHECK the output (otherwise the test will succeed even after symlinking llc to /bin/true :-)

Oct 12 2017, 9:09 AM

Oct 11 2017

aprantl added a comment to D38830: [DWARF] Fix bad comparator in sortGlobalExprs..

Thanks! Since for the testcase only global variables are interesting, you should be able to strip out everything from the IR that is not a global variable and not reachable via a !DIGlobalVariable metadata node. You can probably just run delta on the input (especially if you insert extra newlines after each element in a DIArray (!{!1, !2}).

Oct 11 2017, 4:56 PM

Oct 10 2017

aprantl added a comment to D38473: Include getting generated struct offsets in CodegenABITypes.

I can commit this for you if John is happy with it.

Oct 10 2017, 4:09 PM
aprantl added inline comments to D38719: [llvm-dwarfdump] Verify compatible TAG for attributes..
Oct 10 2017, 10:10 AM · debug-info
aprantl accepted D38719: [llvm-dwarfdump] Verify compatible TAG for attributes..

LGTM with outstanding nitpicks addressed and assuming David is happy with the patch.

Oct 10 2017, 10:09 AM · debug-info
aprantl added inline comments to D38719: [llvm-dwarfdump] Verify compatible TAG for attributes..
Oct 10 2017, 10:07 AM · debug-info
aprantl accepted D38686: [dsymutil] Timestmap verification for __swift_ast.
Oct 10 2017, 7:38 AM

Oct 9 2017

aprantl added inline comments to D38686: [dsymutil] Timestmap verification for __swift_ast.
Oct 9 2017, 4:35 PM