This is an archive of the discontinued LLVM Phabricator instance.

[GlobalISel] Fix legalizer trying to process a deleted instruction
ClosedPublic

Authored by aemerson on Oct 6 2017, 6:05 AM.

Details

Summary

[GlobalISel] Fix legalizer trying to process a deleted instruction.

In some cases an instruction is deleted from the block during combining, however it can still exist in the legalizer worklist.

This change modifies the combiner routines to add the given MI to the dead instruction list rather than trying to remove it from the block themselves. The responsibility is then on the caller to delete the instruction from the block and any worklists.

This bug was exposed by D38621 on a test suite build.

Diff Detail

Repository
rL LLVM

Event Timeline

aemerson created this revision.Oct 6 2017, 6:05 AM

Forgot to add llvm-commits, pasting description again:

[GlobalISel] Fix legalizer trying to process a deleted instruction.

In some cases an instruction is deleted from the block during combining, however it can still exist in the legalizer worklist.

This change modifies the combiner routines to add the given MI to the dead instruction list rather than trying to remove it from the block themselves. The responsibility is then on the caller to delete the instruction from the block and any worklists.

This bug was exposed by D38621 on a test suite build.

aemerson added a reviewer: ab.Oct 6 2017, 8:58 AM
qcolombet accepted this revision.Oct 6 2017, 10:20 AM
qcolombet added a subscriber: qcolombet.

LGTM with one comment: could we have the whole pattern being a helper function?

This revision is now accepted and ready to land.Oct 6 2017, 10:20 AM
This revision was automatically updated to reflect the committed changes.

Thanks, I've put the idiom into a helper and committed. Let me know if it's not what you had in mind.

Cheers,
Amara