Skip to content

Pick fix for matrix lowering crashes #9693

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 3 commits into from
Dec 5, 2024

Conversation

fhahn
Copy link

@fhahn fhahn commented Dec 5, 2024

rdar://140924116

fhahn added 3 commits December 5, 2024 15:36
lowerDotProduct called above may already lower a matrix multiply and
mark it as procssed by adding it to FusedInsts. Don't try to process it
again in LowerMatrixMultiplyFused by checking if FusedInsts.

Without this change, we trigger an assertion when trying to erase the
same original matrix multiply twice.
ValueMap automatically updates entries with the new value if they have
been RAUW. This can lead to instructions that are expected to not have
shape info to be added to the map (e.g. shufflevector as in the added
test case).

This leads to incorrect results. Originally it was used for transpose
optimizations, but they now all use updateShapeAndReplaceAllUsesWith,
which takes care of updating the shape info as needed.

This fixes a crash in the newly added test cases.

PR: llvm#118282
Builder.Create(F)Add may constant fold the inputs, return a constant
instead of an instruction. Account for that instead of crashing.
@fhahn fhahn changed the base branch from next to stable/20240723 December 5, 2024 15:53
@fhahn fhahn requested a review from a team as a code owner December 5, 2024 15:53
@fhahn
Copy link
Author

fhahn commented Dec 5, 2024

@swift-ci please test

@QuietMisdreavus QuietMisdreavus removed their request for review December 5, 2024 15:56
@fhahn fhahn merged commit 16092ea into swiftlang:stable/20240723 Dec 5, 2024
3 checks passed
@fhahn fhahn deleted the matrix-crash branch December 5, 2024 23:23
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.

1 participant