Skip to content

Fix an assertion failure in SILMem2Reg. #6695

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 21, 2017

Conversation

bob-wilson
Copy link
Contributor

The ASI->eraseFromParent() call after removeSingleBlockAllocation(ASI)
will assert if there are any remaining uses of the allocation ASI. This can
happen if there are any dead instructions for struct or tuple address
projections, since removeSingleBlockAllocation only removes the instructions
that are actually used by loads and stores. The existing stdlib/subString.swift
test exposes this issue when running check-swift-optimize.
rdar://problem/28671838

The ASI->eraseFromParent() call after removeSingleBlockAllocation(ASI)
will assert if there are any remaining uses of the allocation ASI. This can
happen if there are any dead instructions for struct or tuple address
projections, since removeSingleBlockAllocation only removes the instructions
that are actually used by loads and stores. The existing stdlib/subString.swift
test exposes this issue when running check-swift-optimize.
rdar://problem/28671838
@bob-wilson
Copy link
Contributor Author

@swift-ci please smoke test

@bob-wilson
Copy link
Contributor Author

@eeckstein Can you please review this?

@slavapestov
Copy link
Contributor

A reduced test case would be nice.

@eeckstein
Copy link
Contributor

LGTM!
But, as Slava said, you should add a test case.

@slavapestov slavapestov self-assigned this Jan 12, 2017
@slavapestov
Copy link
Contributor

On second thought, since this fixes a bug, we can merge it as-is, as long as @bob-wilson adds that new testcase one day :-)

@bob-wilson
Copy link
Contributor Author

Thanks. I'm still hoping to get that testcase this week. If I can't get it done by Friday, I'll go ahead without it.

@slavapestov
Copy link
Contributor

It's Friday! ominous music plays

@bob-wilson
Copy link
Contributor Author

Yeah, I spent all week doing meetings and stuff. I'm looking at the test case now, but I'll merge the fix separately.

@bob-wilson bob-wilson merged commit 0d9ff64 into swiftlang:master Jan 21, 2017
@bob-wilson bob-wilson deleted the mem2reg-assert-fix branch January 21, 2017 05:36
@bob-wilson
Copy link
Contributor Author

Here's a test case: #6987

(Writing the SIL by hand was far easier than I expected and I can't believe I spent so much time trying to reduce a test case from the original input. Sorry it took so long.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants