Page MenuHomePhabricator

krisb (Kristina Bessonova)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 1 2018, 11:54 PM (96 w, 2 d)

Recent Activity

Today

krisb updated the summary of D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.
Sun, Dec 8, 5:46 AM · debug-info, Restricted Project
krisb created D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.
Sun, Dec 8, 5:46 AM · debug-info, Restricted Project
krisb added a comment to D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.

Thank everyone for reviewing this!
Committed as 68f464ac2ef5de8cb2e8beaeee43c435c536539e.

Sun, Dec 8, 4:55 AM · debug-info, Restricted Project
krisb committed rG68f464ac2ef5: [llvm-dwarfdump][Statistics] Unify coverage statistic computation (authored by krisb).
[llvm-dwarfdump][Statistics] Unify coverage statistic computation
Sun, Dec 8, 4:52 AM
krisb closed D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.
Sun, Dec 8, 4:52 AM · debug-info, Restricted Project
krisb added a comment to D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.

@djtodoro , yeah, it doesn't look good.
I also considered an option to rename the first bucket to smth like "No location info" or "Not covered" or something similar then the second can be safely named '0-9%'.
Or simply to use '0-9%' together with '0%' which might be a bit unclear, but as docs say 'The line 0% shows the number and the percentage of DIEs with no location information', it should not be confusing.

Sun, Dec 8, 4:33 AM · debug-info, Restricted Project

Thu, Dec 5

krisb created D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.
Thu, Dec 5, 9:38 AM · debug-info, Restricted Project

Sun, Nov 24

krisb added a comment to D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.

@krisb Thanks for working on this!

Let me summarize this quickly.

This will remove mixing between calculation against enclosing and adjusted scope. We have concluded the adjustment heuristics are confusing and wrong in many cases, so we are giving up on that. The alternative is to generate the DW_AT_start_scope attribute, that represents exactly the adjusted scope, and having that we will be able to calculate the debug location statistics for the pieces of code where the variable is alive.

WDYT how "easy" will be to implement the DW_AT_start_scope attribute? I know that it might not be used anywhere else, but I think that having location statistics against variable's life will be very useful thing.

See my reply from D69027 - though thinking more closely, I'm not sure start_scope would really answer the question as it seems like it has many of the same issues as the adjusted scope we're removing - it assumes the code is laid out in order. Any use of DW_AT_ranges would invalidate it (& I /think/ many & perhaps most scopes are fragmented into ranges in optimized code which is where any of these stats are interesting) & basic block reordering, etc, is sort of the nail in the coffin.

Sun, Nov 24, 10:10 AM · debug-info, Restricted Project
krisb added a comment to D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.

@krisb Thanks for working on this!

Let me summarize this quickly.

This will remove mixing between calculation against enclosing and adjusted scope. We have concluded the adjustment heuristics are confusing and wrong in many cases, so we are giving up on that. The alternative is to generate the DW_AT_start_scope attribute, that represents exactly the adjusted scope, and having that we will be able to calculate the debug location statistics for the pieces of code where the variable is alive.

Sun, Nov 24, 9:42 AM · debug-info, Restricted Project
krisb updated the diff for D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.

Refactored llvm/utils/llvm-locstats/llvm-locstats.py variable's names to avoid mentioning 'first def'.

Sun, Nov 24, 9:23 AM · debug-info, Restricted Project

Thu, Nov 21

krisb updated the summary of D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.
Thu, Nov 21, 11:38 PM · debug-info, Restricted Project
krisb abandoned D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

Abandon the revision as I submitted another one https://reviews.llvm.org/D70548 that removes OffsetToFirstDefinition instead of trying to fix it, as suggested by @avl.

Thu, Nov 21, 10:14 AM · debug-info, Restricted Project
krisb created D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.
Thu, Nov 21, 10:05 AM · debug-info, Restricted Project

Sat, Nov 9

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

When I started to work on the statistics, I also thought that the percentage buckets are to show completeness of the debug info. Unfortunately, due to BB reordering this information, sometimes not just imprecise, but rather terribly wrong. And in general case, we can't even guarantee any precision. I agree with @dblaikie; we need a way to calculate adjusted scope correctly regardless of basic block reordering to justify the notion of the adjusted scope itself. Otherwise, I'd prefer to reduce maintenance costs and to calculate debug info coverage against the full enclosing scope. In such a case, only changes in percentages would make sense, but at least this information won't be misleading. So, I tend to abandon this patch and proceed with another one that changes all the calculations against an adjusted scope to calculations against a full enclosing scope (if there are no strong objections).

Sat, Nov 9, 11:57 AM · debug-info, Restricted Project

Nov 1 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

Currently (in the current master), we have the following statistics related to the discussion:

  1. The total number of bytes covered by DW_AT_location over all variables. We also have separate statistics for local vars only, params only and entry values for vars and params.
  2. The total number of bytes in variable's scopes over all variables. Variable's scope means a scope adjusted to the lowerest address in variables' DW_AT_location. Plus, separate statistics for local vars only and params only.
  3. The number of variables in each coverage bucket, where the coverage bucket calculated as 'the number of bytes covered by a variable's DW_AT_location' / 'the number of bytes in variable's enclosing scope'. Variable's enclosing scope means a full scope (e.g. full subprogram, lexical block, etc.) without any adjustments. There are also separate statistics for local vars and params, and all the three where entry values are excluded.
Nov 1 2019, 4:10 AM · debug-info, Restricted Project

Oct 31 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I see the primary value of this statistic to allow us to see change — then a human has to make judgement what that change means. Regardless which choice we make, we may get a false signal from a compiler change that reorders basic blocks, but we'll get valuable signal when we're testing the ramifications of a debug-info-only change to the compiler. As long as we document what the statistics measure, I think we just need to pick something and run with it.

Sure - then I'd suggest that a good statistic to choose would be "bytes covered by location lists" (which is already there) and maybe, if you want a % one, "bytes covered by location lists" / "bytes of enclosing scopes" (counting each scope once for each variable within it - or doing the loc-list-intersects-with-enclosing-scope earlier & just tracking the relative numbers from that point).

Oct 31 2019, 4:07 PM · debug-info, Restricted Project
krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

@dblaikie thanks, David! This is definitely needed to be kept in mind.

Oct 31 2019, 3:58 PM · debug-info, Restricted Project

Oct 29 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.
In D69027#1723448, @avl wrote:

If the other metric is valuable, I suggest implementing it in a separate patch. But could you give an example when calculating coverage against full scope provides additional knowledge which does not follow from currently existing metrics?

suppose we have such case with incomplete debug info:

DW_TAG_lexical_block

DW_AT_ranges  (0x00000000
   [0x000000000000010, 0x0000000000000197))

DW_TAG_variable
  DW_AT_location  (0x0000018e
     [0x0000000000000183,  0x0000000000000197): DW_OP_reg0 RAX)

And then will create a fix which improves debug info in such a way:

DW_TAG_lexical_block

DW_AT_ranges  (0x00000000
   [0x000000000000010, 0x0000000000000197))

DW_TAG_variable
  DW_AT_location  (0x0000018e
     [0x0000000000000100,  0x0000000000000183): DW_OP_reg1 RBX)
     [0x0000000000000183,  0x0000000000000197): DW_OP_reg0 RAX)

In both cases reported coverage there would be 100%.
That hides original low coverage for that variable and makes invisible performed improvement.
Change in coverage looks like this: 100%->100%. If it instead would be 20%->30% then result of changes would be seen.

Doesn't the absolute number of covered bytes (i.e. ScopeBytesCovered) suite this purpose? Yes, we will see no change in coverage, but the absolute number will be changed from 14 to 97 bytes, which may be considered as an improvement. Does it make sense to you?

Oct 29 2019, 12:25 PM · debug-info, Restricted Project

Oct 27 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.
In D69027#1718400, @avl wrote:

Speaking of variable's enclosing scope: it is necessary to take into account not only OffsetFromFirstDefinition, which points into the start of the variable's enclosing scope. The end of the variable's address ranges should also be encountered.

Oct 27 2019, 4:06 AM · debug-info, Restricted Project
krisb updated the diff for D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

Bumped version 4, updated more tests, added comment for adjustScopeToFirstDef().

Oct 27 2019, 4:06 AM · debug-info, Restricted Project

Oct 21 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

@avl, thanks for the comment.

Oct 21 2019, 3:58 AM · debug-info, Restricted Project

Oct 16 2019

krisb added a comment to D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.

I'm confused - given the presence of noncontiguous ranges, why would the "first def" be an important property?

The existence of this operation is "iffy" to me - I'd expect a general solution that handles all the noncontiguous range chunks equally, so there wouldn't be a special case for the "first def".

Oct 16 2019, 4:23 PM · debug-info, Restricted Project
krisb updated the summary of D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.
Oct 16 2019, 4:09 AM · debug-info, Restricted Project
krisb created D69027: [llvm-dwarfdump][Statistics] Fix calculation of OffsetToFirstDefinition.
Oct 16 2019, 4:06 AM · debug-info, Restricted Project

Oct 7 2019

krisb added a watcher for debug-info: krisb.
Oct 7 2019, 1:50 PM

Sep 11 2019

krisb added a comment to D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

@dnsampaio, many thanks for committing this!

Sep 11 2019, 8:19 AM · Restricted Project, Restricted Project
krisb updated the diff for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

Rebased

Sep 11 2019, 1:39 AM · Restricted Project, Restricted Project

Sep 2 2019

krisb updated the diff for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

Applied the comment: enable 'sha2' and 'aes' only for armv8 A profile, report warning and disable 'crypto' for other archs. Add more tests.

Sep 2 2019, 1:25 PM · Restricted Project, Restricted Project

Aug 30 2019

krisb added a comment to D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

I see. But this doesn't seem like a valid case, does it? Shouldn't we somehow diagnose it to not to silently ignore user-specified options?

Aug 30 2019, 11:43 AM · Restricted Project, Restricted Project
krisb added a comment to D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

@dnsampaio thanks for reviewing this! Could I ask you to commit the patch (since I don't have commit access yet)?

Aug 30 2019, 6:06 AM · Restricted Project, Restricted Project
krisb updated the diff for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

Added 'CPU.empty()' check back.

Aug 30 2019, 6:05 AM · Restricted Project, Restricted Project

Aug 29 2019

krisb added a comment to D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

@dnsampaio, thanks! This is definitely better. I also added ARMV8_5A as a test case.

Aug 29 2019, 3:34 AM · Restricted Project, Restricted Project
krisb updated the diff for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

Applied the comment & rebased.
Also changed the patch to take into account only the latest 'crypto' in the Features vector, to avoid enabling 'sha2' and 'aes' when 'crypto' was finally disabled.

Aug 29 2019, 3:29 AM · Restricted Project, Restricted Project

Aug 15 2019

krisb added a comment to D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

@dnsampaio, thanks, this looks better.

Aug 15 2019, 7:17 AM · Restricted Project, Restricted Project
krisb updated the diff for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.

Applied the comment.

Aug 15 2019, 7:16 AM · Restricted Project, Restricted Project

Aug 9 2019

krisb added reviewers for D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature: SjoerdMeijer, ostannard, labrinea, dnsampaio.
Aug 9 2019, 9:18 AM · Restricted Project, Restricted Project
krisb created D66018: [ARM] Take into account -mcpu and -mfpu options while handling 'crypto' feature.
Aug 9 2019, 9:18 AM · Restricted Project, Restricted Project

Apr 11 2019

krisb abandoned D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts.
Apr 11 2019, 1:22 AM · Restricted Project

Jan 25 2019

krisb added inline comments to D56925: Do not use frame pointer by default for MSP430.
Jan 25 2019, 7:26 AM · Restricted Project, Restricted Project
krisb updated the diff for D56787: [MSP430] Set UseIntegratedAssembler.

Fixed broken test.

Jan 25 2019, 6:52 AM · Restricted Project
krisb added a comment to D56787: [MSP430] Set UseIntegratedAssembler.

@asl thanks for catching this! Looks like I forgot to amend a fix for the test after testing :( sorry for that.

Jan 25 2019, 6:51 AM · Restricted Project

Jan 23 2019

krisb added a comment to D57012: Merge similar target diagnostics for interrupt attribute into one. NFC.

@aaron.ballman yes and yes. Thanks!

Jan 23 2019, 7:28 AM

Jan 22 2019

krisb added a comment to D57012: Merge similar target diagnostics for interrupt attribute into one. NFC.

@aaron.ballman Thanks! Could I ask you to commit the patch? I don't have commit access yet.

Jan 22 2019, 12:59 AM
krisb updated the diff for D57012: Merge similar target diagnostics for interrupt attribute into one. NFC.

Applied the comment.

Jan 22 2019, 12:11 AM

Jan 21 2019

krisb created D57015: [MSP430] Ajust f32/f64 alignment according to MSP430 EABI.
Jan 21 2019, 6:14 AM
krisb created D57012: Merge similar target diagnostics for interrupt attribute into one. NFC.
Jan 21 2019, 4:19 AM

Jan 16 2019

krisb created D56787: [MSP430] Set UseIntegratedAssembler.
Jan 16 2019, 8:15 AM · Restricted Project
krisb created D56785: [MSP430] Fix absolute addressing mode printing in AsmPrinter.
Jan 16 2019, 7:47 AM
krisb added inline comments to D56663: [MSP430] Improve support of 'interrupt' attribute.
Jan 16 2019, 7:32 AM
krisb added a reviewer for D56776: [MSP430] Fix msp430-toolchain.c on Windows (added in rL351228): asl.
Jan 16 2019, 2:03 AM
krisb created D56776: [MSP430] Fix msp430-toolchain.c on Windows (added in rL351228).
Jan 16 2019, 2:02 AM

Jan 15 2019

krisb updated the diff for D56663: [MSP430] Improve support of 'interrupt' attribute.

Addressed feedback.

Jan 15 2019, 1:17 PM
krisb added inline comments to D56663: [MSP430] Improve support of 'interrupt' attribute.
Jan 15 2019, 12:55 PM

Jan 14 2019

krisb created D56664: [MSP430] Emit a separate section for every interrupt vector.
Jan 14 2019, 6:33 AM
krisb updated the summary of D56663: [MSP430] Improve support of 'interrupt' attribute.
Jan 14 2019, 6:31 AM
krisb updated the summary of D56663: [MSP430] Improve support of 'interrupt' attribute.
Jan 14 2019, 6:31 AM
krisb created D56663: [MSP430] Improve support of 'interrupt' attribute.
Jan 14 2019, 6:31 AM
krisb updated the summary of D56658: [MSP430] Add msp430 toochain.
Jan 14 2019, 4:31 AM
krisb added a reviewer for D56658: [MSP430] Add msp430 toochain: asl.
Jan 14 2019, 4:30 AM
krisb created D56658: [MSP430] Add msp430 toochain.
Jan 14 2019, 4:29 AM

Jan 12 2019

krisb created D56634: [MSP430] Recoginze '{' as a line separator.
Jan 12 2019, 3:00 AM

Jan 10 2019

krisb created D56547: [MSP430] Minor fixes/improvements for assembler/disassembler.
Jan 10 2019, 7:05 AM
krisb created D56546: [MSP430] Add missing instruction forms.
Jan 10 2019, 6:37 AM

Dec 21 2018

krisb created D56016: [MSP430] Optimize 'shl x, 8[+ N] -> swpb(zext(x)) [<< N]' for i16.
Dec 21 2018, 11:33 AM

Nov 26 2018

krisb created D54890: [MSP430] Fix crash while lowering llvm.stacksave/stackrestore.
Nov 26 2018, 2:29 AM

Nov 17 2018

krisb updated the diff for D54623: [MSP430] Optimize srl/sra in case of A >> (8 + N).

Applied comments and extended condition with 'ShiftAmount == 8'

Nov 17 2018, 1:28 AM

Nov 16 2018

krisb created D54626: [MSP430] Add RTLIB::[SRL/SRA/SHL]_I32 lowering to EABI lib calls.
Nov 16 2018, 3:55 AM
krisb created D54623: [MSP430] Optimize srl/sra in case of A >> (8 + N).
Nov 16 2018, 3:07 AM
krisb created D54620: [MSP430] Use R_MSP430_16_BYTE type for FK_Data_2 fixup.
Nov 16 2018, 2:18 AM

Nov 15 2018

krisb created D54618: [MSP430] Add support for .refsym directive.
Nov 15 2018, 11:45 PM
krisb created D54582: [MSP430] NFC. Add more tests for ABI and calling convention.
Nov 15 2018, 8:07 AM

Apr 19 2018

krisb updated the diff for D45808: [OpenCL] Add 'denorms-are-zero' function attribute.

Applied the comments

Apr 19 2018, 5:16 AM
krisb created D45808: [OpenCL] Add 'denorms-are-zero' function attribute.
Apr 19 2018, 2:57 AM

Mar 2 2018

Herald updated subscribers of D20758: Support addrspacecast initializers with isNoopAddrSpaceCast.
Mar 2 2018, 3:19 AM

Feb 28 2018

krisb updated the diff for D43809: Add possibility to specify output stream for CompilerInstance.

Reverted unintentional changes again. Sorry for that

Feb 28 2018, 2:01 AM
krisb updated the diff for D43809: Add possibility to specify output stream for CompilerInstance.

@teemperor, thanks! All comments are applied.

Feb 28 2018, 1:55 AM

Feb 27 2018

krisb added a comment to D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts.

This is too aggressive. I have D20758 to do this for noon. Otherwise it should fail for the target to do something about it

Feb 27 2018, 10:22 AM · Restricted Project
krisb added a comment to D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts.

I think the lowering of addrspacecast is target dependent.

Feb 27 2018, 8:29 AM · Restricted Project

Feb 26 2018

krisb added a reviewer for D43809: Add possibility to specify output stream for CompilerInstance: teemperor.
Feb 26 2018, 11:59 PM
krisb updated the diff for D43809: Add possibility to specify output stream for CompilerInstance.

Reverted unintentional changes.

Feb 26 2018, 11:56 PM
krisb created D43809: Add possibility to specify output stream for CompilerInstance.
Feb 26 2018, 11:49 PM
krisb added reviewers for D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts: Anastasia, yaxunl.
Feb 26 2018, 11:01 PM · Restricted Project

Feb 22 2018

krisb updated the diff for D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option.

Updated one more test where attributes became mismatched.

Feb 22 2018, 1:25 AM

Feb 21 2018

krisb added reviewers for D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option: yaxunl, Anastasia.
Feb 21 2018, 5:50 AM
krisb created D43570: [OpenCL] Add '-cl-uniform-work-group-size' compile option.
Feb 21 2018, 5:50 AM

Feb 20 2018

krisb added a comment to D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts.

Ping

Feb 20 2018, 5:41 AM · Restricted Project

Feb 12 2018

krisb added reviewers for D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts: craig.topper, pcc.
Feb 12 2018, 6:27 AM · Restricted Project
krisb created D43186: [AsmPrinter] Allow lowerConstant() to strip address space casts.
Feb 12 2018, 6:16 AM · Restricted Project