[LICM] Salvage DebugInfo from dying Instructions

Authored by gramanas on Sat, Mar 10, 2:13 PM.



LICM deletes trivially dead instructions which it won't attempt to sink.
Attempt to salvage debug values which reference these instructions.

This is similar to rL325660, which did the same thing in the BDCE pass.

Diff Detail

gramanas created this revision.Sat, Mar 10, 2:13 PM
gramanas edited the summary of this revision. (Show Details)Sat, Mar 10, 2:19 PM
vsk added a comment.Mon, Mar 12, 4:06 PM

Thanks for working on this! The change in LICM looks good, but the tests need a little bit of work.

2 ↗(On Diff #137928)

Why does this test need to be updated in addition to sinking.ll? It seems like one test case should be enough to exercise the new functionality.


Generally we try not to hardcode metadata numbers into CHECK lines, because it can make for a brittle test case. We wouldn't want this test to fail if e.g the order in which metadata are emitted changes.

Instead of "!143", writing "{{.*}}" here would be fine.


This used to be a CHECK-NEXT, but now it's a CHECK. That loosens the test, as FileCheck will no longer require that the "ret i32" occur immediately after the "trunc i64" instruction.

Can you move the CHECK-NEXT back to where it was, and add the DEBUGIFY check line after it?

vsk added inline comments.Mon, Mar 12, 4:26 PM

When testing this patch out locally, I found that only dbg.value salvaged in sinking.ll is in the function @test11. In that function, there's a dead getelementptr instruction which LICM deletes. Right before it goes away, the call you added to salvageDebugInfo() rewrites the effect of the GEP as a DIExpression:

call void @llvm.dbg.value(metadata %Ty* @X2, metadata !127, metadata !DIExpression(DW_OP_stack_value))

Could you remove the changes to the @PR18753 function, and add checks to @test11 instead?

gramanas updated this revision to Diff 138197.Tue, Mar 13, 8:10 AM
gramanas marked 4 inline comments as done.
gramanas retitled this revision from [LICM] Salvage DebugInfo from dying Instructions to [LICM] Salvage DebugInfo from dying Instructions.
gramanas edited the summary of this revision. (Show Details)
  • fix redundant debugify test
gramanas edited the summary of this revision. (Show Details)Tue, Mar 13, 8:18 AM
gramanas edited the summary of this revision. (Show Details)Tue, Mar 13, 9:02 AM
vsk accepted this revision.Tue, Mar 13, 9:17 AM

LGTM. Is it all right if I commit this for you, or would you like to set up an account and commit it yourself?

This revision is now accepted and ready to land.Tue, Mar 13, 9:17 AM
vsk added a comment.Tue, Mar 13, 9:24 AM

Ah, I think there's still a problem with the test.


The label to check for should be @test11


The dbg.value should look like: call void @llvm.dbg.value(metadata %Ty* @X2, metadata {{.*}}, metadata !DIExpression(DW_OP_stack_value))

If you run "check-llvm" after making this change, the tests should all pass.

gramanas updated this revision to Diff 138217.Tue, Mar 13, 10:02 AM
gramanas marked 2 inline comments as done.
  • Fix test label
vsk accepted this revision.Tue, Mar 13, 11:55 AM
vsk closed this revision.Sun, Mar 18, 10:26 PM

This landed as r327800. @gramanas, if you add "Differential Revision: <URL to the review>" to the last line of your commit message, Phabricator will automatically close the review for you when you commit.