Page MenuHomePhabricator

krisb (Kristina Bessonova)
User

Projects

User does not belong to any projects.

User Details

User Since
Feb 1 2018, 11:54 PM (113 w, 3 d)

Recent Activity

Jan 28 2020

krisb committed rG5499e2f455ca: [llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name (authored by krisb).
[llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name
Jan 28 2020, 10:59 AM
krisb committed rG4b0a7fe008e6: [llvm-dwarfdump][Statistics] Make calculations of vars in global scope more… (authored by krisb).
[llvm-dwarfdump][Statistics] Make calculations of vars in global scope more…
Jan 28 2020, 10:59 AM
krisb committed rG57839e517815: [llvm-dwarfdump][Statistics] Count more than one conrete out-of-line instances… (authored by krisb).
[llvm-dwarfdump][Statistics] Count more than one conrete out-of-line instances…
Jan 28 2020, 10:59 AM
krisb closed D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate.
Jan 28 2020, 10:58 AM · Restricted Project, debug-info
krisb closed D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function.
Jan 28 2020, 10:58 AM · Restricted Project, debug-info
krisb closed D73003: [llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name.
Jan 28 2020, 10:58 AM · Restricted Project, debug-info
krisb committed rG2e5d20bd4788: [llvm-dwarfdump][Statistics] Ignore declarations of global variables (authored by krisb).
[llvm-dwarfdump][Statistics] Ignore declarations of global variables
Jan 28 2020, 9:52 AM
krisb closed D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables.
Jan 28 2020, 9:52 AM · Restricted Project, debug-info
krisb committed rGe76106e01c4b: [llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics (authored by krisb).
[llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics
Jan 28 2020, 9:52 AM
krisb closed D72983: [llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics.
Jan 28 2020, 9:51 AM · Restricted Project, debug-info
krisb committed rG9806b39dae18: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name… (authored by krisb).
[llvm-dwarfdump][Statistics] Distinguish functions/variables with same name…
Jan 28 2020, 9:51 AM
krisb closed D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 28 2020, 9:51 AM · debug-info, Restricted Project
krisb added a comment to D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

@djtodoro @aprantl thanks for reviewing this whole set of patches!

Jan 28 2020, 6:31 AM · debug-info, Restricted Project

Jan 24 2020

krisb added a comment to D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function.

@aprantl thanks for the review! Are you okay with this test?

Jan 24 2020, 10:13 AM · Restricted Project, debug-info

Jan 22 2020

krisb added inline comments to D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate.
Jan 22 2020, 11:37 AM · Restricted Project, debug-info
krisb updated the diff for D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate.

Refactor checks for constant members a bit.

Jan 22 2020, 11:37 AM · Restricted Project, debug-info
krisb updated the diff for D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function.

Added a test.

Jan 22 2020, 11:19 AM · Restricted Project, debug-info
krisb added a comment to D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables.

@djtodoro thanks! Are you okay with this test case?

Jan 22 2020, 10:42 AM · Restricted Project, debug-info
krisb updated the diff for D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables.

Add a test.

Jan 22 2020, 10:42 AM · Restricted Project, debug-info
krisb added a comment to D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

@djtodoro those two binaries with 99% of availability just have some variables w/o locations (mostly artificial 'this' parameters of member functions).
But this change reveals one more interesting case.
For code like this:

int foo() {
	if (cond1) {
		static const int a = 1;
		...
	}
	if (cond2) {
		static const int a = 1;
		...
	}
}

clang (unlike gcc) generates both a variables in the function scope, not in a corresponded DW_TAG_lexical_block (as gcc does).
I'm not sure clang is right here, cause a - despite it is static - isn't visible outside the if.
What do you think about this?

Jan 22 2020, 8:22 AM · debug-info, Restricted Project
krisb updated the diff for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

Use declaration info only to identify functions, global variables, and members.

Jan 22 2020, 8:12 AM · debug-info, Restricted Project

Jan 20 2020

krisb added a comment to D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

So, this increases number of 'total vars' because these 'a' and 'b' variables counted twice.

I see... Can we do something to avoid the additional calculation? Somehow to recognize we already observed the a and b, although they have different DIE ID.

I also was a bit optimistic about how this patch(es) affects the performance of the statistics, it actually became two times slower than before the patches.

What patch from the stack adds the overhead? This one?

Yeah, this one mostly causes the overhead.
Actually, adding declaration info for ID is only necessary for functions, global variables, and members (because we need to handle static/inline functions/variables, and don't respect namespaces, classes, and structures). For local variables and parameters, Prefix + Name should be enough to distinguish all the cases (because their prefix already guaranteed uniqueness, for parameters the prefix is going to be added in D73003).
This approach shows better results:

Jan 20 2020, 8:13 AM · debug-info, Restricted Project

Jan 19 2020

krisb added a child revision for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs: D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate.
Jan 19 2020, 8:59 AM · debug-info, Restricted Project
krisb added a parent revision for D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate: D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 19 2020, 8:59 AM · Restricted Project, debug-info
krisb created D73004: llvm-dwarfdump][Statistics] Make calculations of vars in global scope more accurate.
Jan 19 2020, 8:59 AM · Restricted Project, debug-info
krisb added a parent revision for D73003: [llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name: D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 19 2020, 8:13 AM · Restricted Project, debug-info
krisb added a child revision for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs: D73003: [llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name.
Jan 19 2020, 8:13 AM · debug-info, Restricted Project
krisb created D73003: [llvm-dwarfdump][Statistics] Distinguish parameters with same name or w/o a name.
Jan 19 2020, 8:13 AM · Restricted Project, debug-info
krisb added a parent revision for D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function: D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 19 2020, 8:03 AM · Restricted Project, debug-info
krisb created D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function.
Jan 19 2020, 8:03 AM · Restricted Project, debug-info
krisb added a child revision for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs: D73002: [llvm-dwarfdump][Statistics] Count more than one concrete out-of-line instance of a function.
Jan 19 2020, 8:03 AM · debug-info, Restricted Project
krisb added a parent revision for D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables: D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 19 2020, 7:08 AM · Restricted Project, debug-info
krisb added a child revision for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs: D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables.
Jan 19 2020, 7:08 AM · debug-info, Restricted Project
krisb created D73001: [llvm-dwarfdump][Statistics] Ignore declarations of global variables.
Jan 19 2020, 7:08 AM · Restricted Project, debug-info

Jan 18 2020

krisb added a parent revision for D72983: [llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics: D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 18 2020, 12:53 PM · Restricted Project, debug-info
krisb added a child revision for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs: D72983: [llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics.
Jan 18 2020, 12:53 PM · debug-info, Restricted Project
krisb created D72983: [llvm-dwarfdump][Statistics] Ignore DW_TAG_subroutine_type in statistics.
Jan 18 2020, 12:53 PM · Restricted Project, debug-info
krisb added a comment to D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

@aprantl @djtodoro thanks for the comments!

Jan 18 2020, 10:13 AM · debug-info, Restricted Project
krisb updated the summary of D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 18 2020, 10:13 AM · debug-info, Restricted Project
krisb updated the diff for D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.

Applied the comments.

Jan 18 2020, 10:07 AM · debug-info, Restricted Project

Jan 15 2020

krisb created D72797: [llvm-dwarfdump][Statistics] Distinguish functions/variables with same name across different CUs.
Jan 15 2020, 12:05 PM · debug-info, Restricted Project

Dec 26 2019

krisb committed rGcdd25a4c7410: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another… (authored by krisb).
[DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another…
Dec 26 2019, 10:06 AM
krisb closed D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.
Dec 26 2019, 10:06 AM · debug-info, Restricted Project
krisb added a comment to D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.

Yeah, the rest of the code around SelectionDAG doesn't do a good job of keeping location intrinsics in order; I think it's acceptable to suffer a little bit more re-ordering here for an increase in available locations, for now.

Okay, I'll commit the patch as is, then. This issue with 'IROrder' in SelectionDAG is quite tricky, so I'll submit further changes in separate patches.

Dec 26 2019, 9:28 AM · debug-info, Restricted Project
krisb updated the diff for D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.

Rebased and reduced the test a bit more.

Dec 26 2019, 8:51 AM · debug-info, Restricted Project

Dec 13 2019

krisb added a comment to D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.

@spatel thanks for reporting this!
Could you, please, check that llvm-locstat wasn't cached in the build/install directory? llvm-locstat should look for a variables with (0%,10%) of its scope covered as was changed by the patch, so I guess it didn't get updated for some reason.

Dec 13 2019, 7:56 AM · debug-info, Restricted Project
krisb committed rGd5655c4d2e18: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0% (authored by krisb).
[llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%
Dec 13 2019, 6:44 AM
krisb closed D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.
Dec 13 2019, 6:44 AM · debug-info, Restricted Project
krisb committed rG1cc4b603ba79: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC (authored by krisb).
[llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC
Dec 13 2019, 5:13 AM
krisb closed D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.
Dec 13 2019, 5:12 AM · debug-info, Restricted Project

Dec 12 2019

krisb updated the summary of D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.
Dec 12 2019, 4:06 AM · debug-info, Restricted Project
krisb updated the diff for D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.

Updated the doc, rebased on D71366.

Dec 12 2019, 3:59 AM · debug-info, Restricted Project
krisb updated the diff for D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.

Fixed one more misprint.

Dec 12 2019, 1:12 AM · debug-info, Restricted Project

Dec 11 2019

krisb added a child revision for D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC: D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.
Dec 11 2019, 11:48 PM · debug-info, Restricted Project
krisb updated the diff for D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.

Fixed a misprint in llvm/docs/CommandGuide/llvm-locstats.rst.

Dec 11 2019, 11:48 PM · debug-info, Restricted Project
krisb added a comment to D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.

@djtodoro , Ah, missed this one. Thank you so much!

Dec 11 2019, 11:48 PM · debug-info, Restricted Project
krisb added a parent revision for D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%: D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.
Dec 11 2019, 11:48 PM · debug-info, Restricted Project
krisb added a comment to D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.

@djtodoro , @dblaikie thanks for your opinions! I created a separate review for this change https://reviews.llvm.org/D71366. Please, take a look if you have a chance.

Dec 11 2019, 10:39 AM · debug-info, Restricted Project
krisb updated the diff for D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.

Fix indentation in llvm-locstats output.

Dec 11 2019, 10:32 AM · debug-info, Restricted Project
krisb created D71366: [llvm-dwarfdump][Statistics] Change the coverage buckets representation. NFC.
Dec 11 2019, 10:20 AM · debug-info, Restricted Project
krisb added a comment to D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.

@aprantl, @djtodoro thanks! I reduced the test a bit more.

Dec 11 2019, 8:49 AM · debug-info, Restricted Project
krisb updated the diff for D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.

Reduced the test.

Dec 11 2019, 8:46 AM · debug-info, Restricted Project

Dec 9 2019

krisb added a comment to D71070: [llvm-dwarfdump][Statistics] Don't count coverage less than 1% as 0%.

@dblaikie , @djtodoro sorry for paying so much attention to details here, but does this one work with you?

Dec 9 2019, 3:15 AM · debug-info, Restricted Project

Dec 8 2019

krisb updated the summary of D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.
Dec 8 2019, 5:46 AM · debug-info, Restricted Project
krisb created D71175: [DebugInfo][SelectionDAG] Change order while transferring SDDbgValue to another node.
Dec 8 2019, 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.

Dec 8 2019, 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
Dec 8 2019, 4:52 AM
krisb closed D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.
Dec 8 2019, 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.

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

Dec 5 2019

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

Nov 24 2019

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.

Nov 24 2019, 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.

Nov 24 2019, 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'.

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

Nov 21 2019

krisb updated the summary of D70548: [llvm-dwarfdump][Statistics] Unify coverage statistic computation.
Nov 21 2019, 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.

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

Nov 9 2019

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).

Nov 9 2019, 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