- User Since
- Aug 17 2017, 6:34 AM (126 w, 2 d)
Anyway, it seems like you could do away with a fair amount of the complexity in this patch for handling guarding musttail forwarding by declaring them to be an "explicit FP use", since it is not, in general, possible for a perfectly forwarding thunk to know if XMM arguments have been used.
Wed, Jan 15
Tue, Jan 14
Mon, Jan 13
Sun, Jan 12
style issues reported by clang-tidy this time does not relate to my patch.
Sat, Jan 11
apply clang-tidy findings.
Thu, Jan 9
Wed, Jan 8
Tue, Jan 7
apply clang-tidy/clang-format changes.
Mon, Jan 6
Colleagues, any comments on this review?
Sat, Jan 4
Fri, Jan 3
lgtm with a small suggestion
addressed comments(enumerate usages and create replacement variables for only used elements).
My personal preference goes to lib/DWARFLinker.
Mon, Dec 30
I started from the implementation similar to above description. But later I decided to avoid global SRA for big arrays, since big array with small number of usages is kind of artificial case. It is probably not worth to care about it too much. But, since that implementation was requested - I will update this patch with alternative implementation navigating through the usages and not-creating huge number of unused variables...
above two failures do not relate to my fix. Patch which introduced them is already reverted : https://reviews.llvm.org/rG32cc14100e802fddd9f88e7a862250ce3108a583
Tue, Dec 24
Not sure if this is better/worse off as a subdirectory of lib/CodeGen, rather than as a top level library on its own (lib/DWARFOptimizer). Figured I'd mention it, but mostly leave it up to the other dsymutil folks to make the choices here.
Mon, Dec 23
Dec 19 2019
Dec 18 2019
Dec 17 2019
Dec 16 2019
Dec 13 2019
Dec 12 2019
Dec 11 2019
addressed comments(changed test, used an asm test)
addressed comments(removed clang part, reworked testcase).
Dec 10 2019
Dec 9 2019
Dec 6 2019
Hi @nemanjai I has recently integrated the fix for that problem https://reviews.llvm.org/rG8c714c93023d7d039a23fb47c8256570ba54b9c7 . I am sorry for inconvenience.
I integrated the fix which cures building of shared libraries - https://reviews.llvm.org/rG8c714c93023d7d039a23fb47c8256570ba54b9c7 . I would additionally check whether it is working now for all above settings. Thank you.
@gchatelet This patch seems to break my build:
Dec 5 2019
Dec 4 2019
The modules build is good because it's kind of a special case, but I'd like to check a clang without modules as well. I'm a little confused by LLVM_USE_SPLIT_DWARF though, as that's not something that makes sense on macOS unless it's a NO-OP?
Dec 3 2019
@avl - are you planning to do the DW_TAG_split_compile/DW_TAG_split_type tags required by DWARFv5 too in separate patches?
Given that this should NFC, can you please run dsymutil with and without your change on clang and ensure the MD5 hash is identical?
Dec 2 2019
resubmitted this review - D70880
Dec 1 2019
Nov 26 2019
Hi Konstantin, That review looked stuck for a long time.
I tried to keep all your original authority - ping original review,
waited for response for a week, added link to your review into the new review,
put your authority in the commit message.
Apologies for not waiting longer.
Hi Konstantin, yes, I had in mind something like this. Unfortunately, I noticed your answer after D70652 was integrated : https://reviews.llvm.org/rGe73f78acd34360f7450b81167d9dc858ccddc262
Nov 25 2019
addressed comment. applied clang-format. Thanks!
Nov 24 2019
@krisb the patch looks good to me. though I would prefer if someone from debuginfo project would approve it. Thank you.
Nov 21 2019
Nov 18 2019
@kbelochapka Hi Konstantin, I am not sure whether you are still waiting for that review ... But if that is the case, Would you consider following comments, please ?
Nov 17 2019
Nov 11 2019
addressed comments(moved expansion of xmm registers code until after register allocator).
Nov 7 2019
Is it possible to avoid expanding VARARG_THUNK_SAVE_XMM_REGS until after register allocation? I would rather not make the liveness rules more complicated just for the sake of working around a limitation of fast regalloc.
Nov 6 2019
I'm not sure the bleakness is as much of a problem (but I didn't implement/don't personally use the statistics at all) - like all the other measures, this would be a relative one, not one where "100%" was a goal/meaningful (& the only reason to view it as a % would be to understand progress in a relative sense, I guess? Though measuring it on exactly the same code as an absolute value would be fine and represent progress too)
Nov 5 2019
fix small typo introduced by previous update.
removed formatting done by clang-format for X86ISelLowering.h. ping...
Nov 1 2019
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
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.
Oct 29 2019
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?
I'm not sure this answers the question which was about your statement: "The end of the variable's address ranges should also be encountered."
Oct 28 2019
I have my patch based on two assumptions:We only care about lexically (or statically) scoped languages. Otherwise detecting a variable's scope is very problematic. In a lexically scoped language the end of a variable's scope should match the end of the enclosing lexical scope.
That's why I a bit confused about why would we need to make some adjustments to the end of a range. Could you kindly clarify what did you mean?