Skip to content
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

mirpasses: split assignments with MIR pass #1366

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Jun 30, 2024

Summary

  • use MIR pass to split assignments involving potentially raising calls
  • for the C backend, the compiler employing RVO no longer affects
    observable behaviour

Details

  • add the splitAssignments MIR pass and enable it for the C backend
  • the condition of when to split assignments is the same as
    ccgcalls.isHarmlessStore, with the difference that
    splitAssignments also considers projections of locals when
    optimizing away a split (not only unprojected locals)
  • ccgcalls can now assume that all assignments are safe; everything
    related to assignment splitting is removed

Observable stores

When the RHS of an assignment was an RVO-using call, the assignment was
not split, resulting in an "observable store" if the call raised an
exception after having modified its result variable.

The splitAssignments call splits all assignments, incurring an
additional memory copy for RVO-using calls but also making the
behaviour "correct". reportObservableStore is obsolete and thus
removed.

zerbina added 3 commits June 30, 2024 22:51
The pass' implementation is a straightforward port of what
`ccgcalls.isHarmlessStore` (plus the subsequent logic) does.
The MIR pass handles this now. `isHarmlessStore` and all usages thereof
are removed.

In addition, `reportObservableStore` is removed. Since "dangerous"
assignments are always split by the MIR pass, "observable stores" are
no longer possible.
The test now works as it should.
@zerbina zerbina added compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features. labels Jun 30, 2024
@zerbina zerbina added this to the MIR phase milestone Jun 30, 2024
@saem
Copy link
Collaborator

saem commented Jun 30, 2024

/merge

Copy link

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

@chore-runner chore-runner bot added this pull request to the merge queue Jun 30, 2024
Merged via the queue into nim-works:devel with commit 216a559 Jul 1, 2024
31 checks passed
@zerbina zerbina deleted the mir-based-assignment-splitting branch July 7, 2024 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/backend Related to backend system of the compiler simplification Removal of the old, unused, unnecessary or un/under-specified language features.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants