- User Since
- Nov 19 2018, 1:12 AM (106 w, 4 d)
Tue, Dec 1
Addressing the issues with use of stored SCEV after IR had been deleted (see https://reviews.llvm.org/D91711).
Fri, Nov 27
Thanks for the clarification. The offending patch has been reverted now (in 808fcfe5944755f0).
Thu, Nov 26
Right, there is nothing special with llvm.dbg.value besides that it is a debug intrinsic so it is not allowed to affect optimizations and the fact that such intrinsic uses another Value may not inhibit transformation on that Value. I just wanted to point it out since the situation that predecessors of an instruction get removed without the instruction itself is a bit unusual (but otherwise unimportant for this discussion).
Wed, Nov 25
Mon, Nov 23
Wed, Nov 18
Oct 29 2020
Oct 21 2020
Some nits below but in general looks good to me.
Oct 8 2020
Oct 6 2020
@nikic, @saugustine, thanks for reverting and having a look. I did not receive any notification mail about those build-bot failures AFAICT. I got two but one failure went away on a subsequent run and the other complained about internal compiler error in g++ which seemed unrelated. Anyway I will try to reproduce locally and address the issue as suggested. Thanks again.
Oct 5 2020
Oct 1 2020
Really minor changes since last approved revision but I would prefer if someone gave a renewed blessing.
Tuns out that previous "safety code" caused problems when bootstraping clang. Apparently it is not safe to do getType() on all SCEVs so instead we now store away the type information and compare on the Value instead of the SCEV.
Sep 30 2020
Added safety code to make sure that we are comparing SCEVs of the same Type.
Sep 29 2020
Addressed minor comments by @bjope
Sep 23 2020
Sep 22 2020
Fixed most of the review remarks.
Sep 21 2020
Updated with full context patch.
Sep 15 2020
Sep 14 2020
Change of appraoch. Instead of hooking into deleteDeadPHIs with an AboutToDeleteCallback we do a pre-pass to store a way the llvm.dbg.value of the loop as well as their SCEV expressions. After LSR has done its thing we go through those stored away llvm.dbg.value and if any of them now has an undef variable location we try to update it using its stored SCEV.
Sep 11 2020
Sep 10 2020
Jun 4 2019
What happens if a target decides late (like addPreEmitPass() late) that a hardware loop is no longer possible due to reasons that are not detectable from the IR level hook. For PowerPC this does not seem to be an issue, or at least I cannot find it in the code, but for other targets there could be limitations that you need to analyse the actual machine instructions to find out about. Since the original IV chain has been replaced by intrinsic llvm.loop.decrement.reg (or whatever it iselected to) it seems we now would need to manually insert instructions to update the loop counter which could possibly be a bit hairy to do this late.
Jun 3 2019
May 21 2019
This is interesting. Our (Ericsson's) out-of-tree target has hardware loops and we currently do a similar thing i.e.
May 13 2019
The DW_OP_LLVM_convert stuff looks-good-to-me but I don't feel that I have authority to approve for the rest so please don't wait for any input from my part.
May 9 2019
May 6 2019
May 3 2019
Superseded by https://reviews.llvm.org/D61499
Apr 30 2019
Apr 29 2019
Can (or should) I proceed to push this again? I am a bit confused since the accepted status remains even though I upload new patches.
Apr 26 2019
Have you verified that this results in the desired DW_OPs being emitted in the end and that the (or a) debugger makes sense out of it?
Use MI.isIndirectDebugValue() and improved the test a bit.
Apr 25 2019
After studying the reproducer and reading some in https://llvm.org/docs/SourceLevelDebugging.html I realize that maybe isImplicit() is not a sufficient condition and that we also need to include the second operand of the DBG_VALUE in the test.
Apr 18 2019
! In D59687#1468571, @probinson wrote:
It sounds like you're trying to do this with aggregate types, and that won't work. Only base types (for DWARF 5) or address-like types (for DWARF <= 4) should end up on the stack.
Apr 16 2019
The Chromium compilation segfault comes down to two issues:
Apr 13 2019
Has been reverted in
commit 0366e3e18142466e4dd99d3109d8facd093cedc8 (llvm.org/master, master) Author: Hans Wennborg <firstname.lastname@example.org> Date: Fri Apr 12 12:54:52 2019 +0000
Apr 12 2019
Apr 11 2019
Moved isImplicit() to .cpp file.
Apr 9 2019
Rebased and updated isImplicit() to only check last elements.
Apr 5 2019
Ping @aprantl, just a friendly reminder.
Mar 28 2019
Rebased and fixed style nit.
Moved isImplicit to DIExpression class and updated test to do sext instead of zext.
Mar 26 2019
Again (and sorry for possibly over emphasizing this) the purpose of this patch is that a DW_OP_deref needs to be inserted before a memory location description can be prepended to an already existing implicit location description as otherwise the latter expression would act on the former address and not the actual value in memory.
Mar 25 2019
I am a bit worried about the other DW_OP_deref used in the compiler and how they behave wrt big-endian targets when loading something that is smaller than the size of an address on the target machine. I do not have any concrete example of where it goes wrong but I have not actively looked for one either.
Mar 22 2019
Mar 20 2019
Mar 19 2019
I guess what we want to achieve here (and please correct me if I am wrong) is
Mar 12 2019
Any improvement is an improvement so I am happy with that but it is still mentioned that this solution is a hack and I guess the
Mar 1 2019
We're almost there!
Feb 28 2019
- Removed explicit check for DW_OP_convert and replaced with a generic handling of operands that should work with all types (regardless if it is first, second or both that is BaseTypeRef).
Feb 27 2019
- Rewrote the expression parsing state machine in DwarfDebug::emitDebugLocEntry to use DWARFExpression as this avoids duplication of code describing ops and their arguments (got inspired by the dsymutil review).
Feb 26 2019
Feb 25 2019
Feb 22 2019
- Added test with two CUs.
- Updated some comments.
- Removed the -generate-typed-dwarf5-expr option, will only produce DW_OP_convert for Dwarf v5, for all prior versions the mask & shift expression is used. I suggest that we forget about DW_OP_GNU_convert and debugger tunings for this review. If that is really desired it can be added later in a separate patch.
Feb 21 2019
- updated to top of trunk to have the llvm-dwarfdump updates committed earlier today
- updated tests and made additional improvements to llvm-dwarfdump
- fixed segfault bug where we crashed on release builds but not debug builds (the allocation of DIEBaseTypeRef)