debug-infoProject
ActivePublic

Recent Activity

Yesterday

aprantl added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

I have no problem with option (3).

Wed, Jun 20, 3:50 PM · debug-info
probinson added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

As I had noted earlier, once you modify the pass so that the debug instruction doesn't interfere with the code motion, the associated debug instructions have to move with the real instruction. In that respect, the patch is doing the right thing.

Wed, Jun 20, 3:19 PM · debug-info
mattd added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

@aprantl Thanks for example.

Wed, Jun 20, 2:59 PM · debug-info
probinson added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

If we were to reorder the instruction stream so that the two intrinsics are swapped, it will look like the value of p is NULL throughout the rest of the program.

Wed, Jun 20, 12:31 PM · debug-info
mattd added inline comments to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
Wed, Jun 20, 12:24 PM · debug-info
aprantl added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
  • the optimizer should never move a debug intrinsic over another intrinsic

Sorry, can you expand on that side of the argument?

Wed, Jun 20, 11:45 AM · debug-info
probinson added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.
  • the optimizer should never move a debug intrinsic over another intrinsic
Wed, Jun 20, 10:49 AM · debug-info
probinson added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

Moving DBG_VALUE instructions along with the instructions that define the associated values is the right thing to do.

Wed, Jun 20, 10:44 AM · debug-info
aprantl added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

That's an interesting question. I could see both sides of the argument:

Wed, Jun 20, 10:35 AM · debug-info
bjope updated subscribers of D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

Just pinging this. @bjope has provided great feedback (thanks!), but I am wondering if anyone else thinks this patch is useful, or if I should abandon it in favor of some other solution?

Wed, Jun 20, 10:13 AM · debug-info
mattd added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

Just pinging this. @bjope has provided great feedback (thanks!), but I am wondering if anyone else thinks this patch is useful, or if I should abandon it in favor of some other solution?

Wed, Jun 20, 10:00 AM · debug-info
aprantl added a comment to D44016: [LiveDebugValues] Track transferring variable's value from one register to another.

Thanks, this is looking pretty good now. I just have a few more small details, mostly to make it easier to read and understand what's going on.

Wed, Jun 20, 8:48 AM · debug-info
aprantl added inline comments to D44016: [LiveDebugValues] Track transferring variable's value from one register to another.
Wed, Jun 20, 8:47 AM · debug-info
NikolaPrica added a comment to D44016: [LiveDebugValues] Track transferring variable's value from one register to another.

@aprantl do you have any more concerns or is this ready for commit?

Wed, Jun 20, 5:06 AM · debug-info

Thu, Jun 14

bjope added a comment to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

An assertion introduced by this patch is failing on a build bot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/25013

Please take a look.

Thu, Jun 14, 9:07 AM · debug-info
morehouse added a comment to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

An assertion introduced by this patch is failing on a build bot: http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-autoconf/builds/25013

Thu, Jun 14, 9:04 AM · debug-info
probinson closed D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Thu, Jun 14, 6:43 AM · debug-info
bjope closed D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.
Thu, Jun 14, 4:28 AM · debug-info

Wed, Jun 13

andrew.zhogin added a comment to D47369: [DebugInfo][ScheduleDAGInstrs] Prevent scheduler from reordering DBG_VALUE instructions through their clobbering MIs.

Ping.

Wed, Jun 13, 3:58 PM · debug-info
JDevlieghere accepted D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..

LGTM!

Wed, Jun 13, 1:09 PM · debug-info
probinson added inline comments to D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Wed, Jun 13, 12:27 PM · debug-info
probinson updated the diff for D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..

Privatize the flags for tracking MD5 usage.

Wed, Jun 13, 12:24 PM · debug-info
JDevlieghere added inline comments to D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Wed, Jun 13, 11:27 AM · debug-info
probinson added inline comments to D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Wed, Jun 13, 11:11 AM · debug-info
probinson updated the diff for D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..

Simplify bool updates.

Wed, Jun 13, 11:10 AM · debug-info
aprantl added inline comments to D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Wed, Jun 13, 10:13 AM · debug-info
aprantl accepted D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

Still works for me, but please add a comment explaining why getStoreSize is used here and what that means.

Wed, Jun 13, 10:10 AM · debug-info
probinson created D48135: [DWARFv5] Tolerate files not all having an MD5 checksum..
Wed, Jun 13, 9:53 AM · debug-info
mattd added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

That's a great explanation, thank you!

Wed, Jun 13, 9:37 AM · debug-info
NikolaPrica added a comment to D44016: [LiveDebugValues] Track transferring variable's value from one register to another.

Ping.

Wed, Jun 13, 8:17 AM · debug-info
bjope added inline comments to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.
Wed, Jun 13, 5:48 AM · debug-info
bjope updated the diff for D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

After some more testing I discovered a case when the added assert in

void llvm::ConvertDebugDeclareToDebugValue(DbgInfoIntrinsic *DII,
                                           PHINode *APN, DIBuilder &Builder)

failed.

Wed, Jun 13, 5:43 AM · debug-info
bjope added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

@bjope , Thanks for your review and example! I agree that any changes to improve performSink/collectDebugValues should probably be in another patch.

I agree, after spending some time with your hypothetical case, I can imagine there are some other cases that might result in old/intermediate values in the user's variable, at least for a short period of time.

...
renamable $eax = COPY $edi
DBG_VALUE debug-use $eax, debug-use $noreg, !14, !DIExpression(), debug-location !16
$esi = ADD $edi, 7
DBG_VALUE debug-use $esi, debug-use $noreg, !14, !DIExpression(), debug-location !17
...

Excuse me if I am getting this wrong, the following is my interpretation of your example. Let's assume this example lives in bb0. In the example, sticking with x86_64, the ADD (or ADD32ri) instruction expects tied physical registers, something like $edi = ADD32ri $edi, 7 However, that case would prevent the COPY from sinking, I'm guessing due to a register def. If we were to make this $esi = ADD32ri $esi, 7, that would allow for the COPY to be sunk. Variable !14 would truly be $esi +7 at that program point in bb0, because the DBG_VALUE you introduced will associate $esi as the value for !14. The sunken copy, presumably in bb1, would be the value copied from $edi, and probably the value that a user would expect. Single stepping from the ADD to the sunken COPY would show !14 as being an intermediate or 'old' value until the sunken copy is reached. This doesn't seem wrong to me. It just could present unexpected values to the user, when inspecting !14 between the ADD and sunken COPY. Perhaps, if we were to delete just the DBG_VALUE associated to the ADD in bb0 , perhaps that would encourage a debugger to present 'optimized-out' in this case. I agree with you, any refinements to performSink or collectDebugValues should probably be for another patch. Thanks again for your review.

Wed, Jun 13, 12:11 AM · debug-info

Tue, Jun 12

mattd added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

@bjope , Thanks for your review and example! I agree that any changes to improve performSink/collectDebugValues should probably be in another patch.

Tue, Jun 12, 3:03 PM · debug-info
thegameg removed a reviewer for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink: aprantl.
Tue, Jun 12, 12:04 PM · debug-info
thegameg added a reviewer for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink: aprantl.
Tue, Jun 12, 12:01 PM · debug-info
probinson closed D48055: [DWARFv5] llvm-mc -dwarf-version does not imply -g..
Tue, Jun 12, 9:13 AM · debug-info
aprantl accepted D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

Few more small nitpicks inline but otherwise LGTM.

Tue, Jun 12, 9:12 AM · debug-info
probinson added a comment to D48055: [DWARFv5] llvm-mc -dwarf-version does not imply -g..

So llvm-mc performs a literal translation by default and with -g it generates debug info for the assembler input? Seems reasonable.

Tue, Jun 12, 9:11 AM · debug-info
bjope added a comment to D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

Basically looks good to me now, as I think that getting rid of the debug-invariant problem is important.
When it comes to the sinking of DBG_VALUE instructions I believe that will be as good as the pre-RA version of MachineSinking. I think that could be enough to land this (as getting rid of the debug-invariance probably is more important).

Tue, Jun 12, 6:07 AM · debug-info
bjope updated the diff for D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

More updates after review.

Tue, Jun 12, 5:13 AM · debug-info

Mon, Jun 11

frederik-h added a comment to D40778: [DebugIR] Revive the Debug IR pass. [Added llvm-commits].

@bollu Are you still interested in working on this? I'm interested in getting this pass back into upstream LLVM. If you don't have time I'm willing to take over this if you like.

Mon, Jun 11, 11:10 PM · debug-info
aprantl accepted D48055: [DWARFv5] llvm-mc -dwarf-version does not imply -g..

So llvm-mc performs a literal translation by default and with -g it generates debug info for the assembler input? Seems reasonable.

Mon, Jun 11, 5:56 PM · debug-info
probinson created D48055: [DWARFv5] llvm-mc -dwarf-version does not imply -g..
Mon, Jun 11, 3:01 PM · debug-info
mattd updated the diff for D45637: [DebugInfo] Ignore DBG_VALUE instructions in PostRA Machine Sink.

Thanks for the review @bjope!

Mon, Jun 11, 10:46 AM · debug-info
aprantl added inline comments to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.
Mon, Jun 11, 10:15 AM · debug-info
bjope updated the diff for D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

Additional cleanups.

Mon, Jun 11, 10:02 AM · debug-info
bjope updated the diff for D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.

Updated after comments from aprantl.

Mon, Jun 11, 9:59 AM · debug-info
aprantl added inline comments to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.
Mon, Jun 11, 9:26 AM · debug-info
bjope added inline comments to D48024: [DebugInfo] Check size of variable in ConvertDebugDeclareToDebugValue.
Mon, Jun 11, 9:03 AM · debug-info