This is an archive of the discontinued LLVM Phabricator instance.

[ArgumentPromotion] Change use of removed argument in llvm.dbg.value to undef
ClosedPublic

Authored by uabelho on Jun 30 2017, 4:45 AM.

Details

Summary

This solves PR33641.

When removing a dead argument we must also handle possibly existing calls
to llvm.dbg.value that use the removed argument. Now we change the use
of the otherwise dead argument to an undef for some other pass to cleanup
later.

If the calls are left untouched, they will later on cause errors:
"function-local metadata used in wrong function"
since the ArgumentPromotion rewrites the code by creating a new function
with the wanted signature, but the metadata is not recreated so the new
function may then erroneously use metadata from the old function.

Diff Detail

Event Timeline

uabelho created this revision.Jun 30 2017, 4:45 AM

Not sure who should review this so I added the last three persons who changed ArgumentPromotion.cpp

rnk added inline comments.Jun 30 2017, 8:27 AM
lib/Transforms/IPO/ArgumentPromotion.cpp
134

There's no reason to believe the only uses of the argument will appear in the entry block. I think you really want to do I->replaceAllUsesWith(undef) to replace all metadata uses of the argument. Some other pass (instcombine?) can clean up the useless dbg.value instructions. That's what the dead argument elimination pass does, anyway.

uabelho updated this revision to Diff 105039.Jul 3 2017, 1:43 AM
uabelho retitled this revision from [ArgPromotion] Remove llvm.dbg.value on removed argument to [ArgumentPromotion] Change use of removed argument in llvm.dbg.value to undef.
uabelho edited the summary of this revision. (Show Details)

Changed according to rnk's comments.

Thanks!

uabelho marked an inline comment as done.Jul 3 2017, 1:44 AM
mstorsjo resigned from this revision.Jul 5 2017, 8:39 AM
rnk accepted this revision.Jul 7 2017, 3:09 PM

lgtm

lib/Transforms/IPO/ArgumentPromotion.cpp
128

This shorter comment should suffice:

// There may be remaining metadata uses of the argument for things like llvm.dbg.value. Replace them with undef.

I think the reason this is special is that for some reason the Function isn't being deleted (is it leaked?), and the Argument isn't being deleted, which would trigger removal from the ValuesAsMetadata map.

This revision is now accepted and ready to land.Jul 7 2017, 3:09 PM
uabelho updated this revision to Diff 105806.Jul 9 2017, 10:15 PM

Updated code comment.

uabelho marked an inline comment as done.Jul 9 2017, 10:16 PM

Thanks! I'll submit this now then.

This revision was automatically updated to reflect the committed changes.