Skip to content

Document ABI registers #25123

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 9 commits into from
May 31, 2019
Merged

Document ABI registers #25123

merged 9 commits into from
May 31, 2019

Conversation

johnno1962
Copy link
Contributor

Hi Apple,

A minor change to ABI manifesto to document register usage in the Swift ABI as it doesn’t seem to be documented elsewhere. If it is, just let me know!

Thanks,

John

@jckarter
Copy link
Contributor

Thanks! This might be more discoverable if we put it in a doc under docs/ABI, maybe a new one just for the calling convention details.

@johnno1962
Copy link
Contributor Author

NP. Having it close to discussion of the context/error regs seemed the most discoverable to me.

@jckarter
Copy link
Contributor

Maybe that whole section of the document could be extracted into a new document(s) under the ABI directory? cc @rjmccall

@johnno1962
Copy link
Contributor Author

Up to you, I confess I looked in the ABI directory when I was trying to puzzle this out.

@johnno1962 johnno1962 force-pushed the reg-docco branch 3 times, most recently from dfaad6e to 24b1f97 Compare May 29, 2019 22:10
@jckarter
Copy link
Contributor

@rjmccall or @aschwaighofer should probably look over this for accuracy too.

@rjmccall
Copy link
Contributor

"Register allocation" is an unfortunate name for this document. Maybe it could just be an appendix in CallingConvention.rst for all the target-specific CC choices?

That file needs some work, too. Among other things, it needs to be converted to Markdown.

@jckarter
Copy link
Contributor

@rjmccall I think we should migrate it into subdocuments under the ABI subdirectory too. I think it'd be more discoverable, and easier for implementers to find the specifics they're looking for, there.

@rjmccall
Copy link
Contributor

@rjmccall I think we should migrate it into subdocuments under the ABI subdirectory too. I think it'd be more discoverable, and easier for implementers to find the specifics they're looking for, there.

By "it" you mean CallingConvention.rst? I agree it should move into ABI/; I'm not sure how it would be useful to split up into different files, but if you have an idea, go ahead.

I suppose we should make a decision about whether target-specific information should be split into appendices of a bunch of different files or unified in a single file per target. The disadvantage of the former is that we're more likely to forget to include information about how a particular target implements a particular ABI feature; the disadvantage of the latter is that the different targets are likely to diverge in terms of the information they include in their documents and how they present it.

@jckarter
Copy link
Contributor

By "it" you mean CallingConvention.rst? I agree it should move into ABI/; I'm not sure how it would be useful to split up into different files, but if you have an idea, go ahead.

Looking at it now, I think it's fine as a single document. I think I had CallingConvention.rst confused with the more catholic ABIStabilityManifesto doc.

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; any other comments @rjmccall @stephentyrone ?

@jckarter
Copy link
Contributor

@swift-ci Please smoke test

@rjmccall
Copy link
Contributor

This is fine; I'd still like to reach agreement about the point I raised above, but we don't need to pretend that this commit will be the last word.

@jckarter jckarter merged commit 269d306 into swiftlang:master May 31, 2019
@johnno1962
Copy link
Contributor Author

Thanks everyone 👍

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.

5 participants