Page MenuHomePhabricator

[SelectionDAG] Fix for PR30775: Assertion `NodeToMatch->getOpcode() != ISD::DELETED_NODE && "NodeToMatch was removed partway through selection"' failed.
ClosedPublic

Authored by ABataev on Jan 30 2017, 11:07 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

ABataev created this revision.Jan 30 2017, 11:07 AM
RKSimon edited edge metadata.Jan 30 2017, 3:12 PM

Can the test case be reduced any further?

test/CodeGen/X86/dag-update-nodetomatch.ll
13 ↗(On Diff #86312)

Add a comment explaining what the test is testing.

ABataev marked an inline comment as done.Jan 31 2017, 1:14 AM

Can the test case be reduced any further?

It is already reduced. I tried to reduce the reproducer, this is the best I could do.

test/CodeGen/X86/dag-update-nodetomatch.ll
13 ↗(On Diff #86312)

Ok, will do

Please can you check if this fixes PR31536 as well?

ABataev marked an inline comment as done.Jan 31 2017, 7:54 AM

Please can you check if this fixes PR31536 as well?

Yes, fixes

Please can you check if this fixes PR31536 as well?

Yes, fixes

Is it worth adding that test case as well?

Please can you check if this fixes PR31536 as well?

Yes, fixes

Is it worth adding that test case as well?

Ok, will add

ABataev updated this revision to Diff 86444.Jan 31 2017, 9:20 AM
ABataev edited the summary of this revision. (Show Details)

Added test from PR31536 + simplified checks + some small changes.

Pity the test cases don't reduce any further...

test/CodeGen/X86/dag-update-nodetomatch.ll
13 ↗(On Diff #86312)

Please can you add the bugzilla PR number to the comments for each test?

ABataev marked an inline comment as done.Feb 1 2017, 7:32 AM
ABataev added inline comments.
test/CodeGen/X86/dag-update-nodetomatch.ll
13 ↗(On Diff #86312)

Ok, will do

ABataev updated this revision to Diff 86641.Feb 1 2017, 8:08 AM
ABataev marked an inline comment as done.

Added comments with PR numbers to the test

Other comments?

RKSimon accepted this revision.Feb 3 2017, 2:19 AM

If the test cases won't reduce any further then I guess this is acceptable.

This revision is now accepted and ready to land.Feb 3 2017, 2:19 AM
This revision was automatically updated to reflect the committed changes.