Page MenuHomePhabricator

[DebugInfo] Ensure that a demanded bits optimisation in InstCombine does not result in an incorrect debuginfo variable value
ClosedPublic

Authored by chrisjackson on Mar 26 2020, 8:35 AM.

Details

Summary

See Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=44371.

A demanded bits optimisation could result in an incorrect debuginfo variable value for a constant operand. This patch attempts to salvage the value before the optimisation is applied.

Diff Detail

Event Timeline

chrisjackson created this revision.Mar 26 2020, 8:35 AM
chrisjackson added projects: debug-info, Restricted Project.Mar 26 2020, 9:18 AM

LGTM but since we're on the same team (now, woot!) I'd like to give others a chance to have a look.

llvm/test/DebugInfo/X86/instcombine-demanded-bits-salvage.ll
13

As we discussed offline the fact that this dbg.value later becomes undef is unrelated to the behaviour you're checking for (a correct salvage operation) so it'd probably be helpful to either add a comment explaining this, or omit the first dbg.value operand in the CHECK line.

aprantl accepted this revision.Mar 26 2020, 9:52 AM

LGTM but since we're on the same team (now, woot!) I'd like to give others a chance to have a look.

Just pointing out that there's no such requirement, but it never hurts to have someone look at a patch who brings in a different perspective.

I did find a bug in your test :-)
Otherwise this looks good!

llvm/test/DebugInfo/X86/instcombine-demanded-bits-salvage.ll
39

You missed a ':' here!
Also, I would avoid hardcoding the metadata numbers wherever possible. Makes the tests hard to maintain.

This revision is now accepted and ready to land.Mar 26 2020, 9:52 AM

Simplified the CHECK expression to match only the component of the dbg.value call that is significant for this test.

Removed unnecessary CHECK line. It was erroneous anyway!

djtodoro accepted this revision.Mar 26 2020, 10:01 AM

LGTM, thanks!

This revision was automatically updated to reflect the committed changes.