Skip to content

[RemoteMirror] Get spare bit info from reflection records #40906

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 42 commits into from
Feb 22, 2022

Conversation

tbkka
Copy link
Contributor

@tbkka tbkka commented Jan 19, 2022

This adds a new reflection record type carrying spare bit information for multi-payload enums.

The compiler includes this for any type that might need it in order to accurately reflect the contents of the enum. The RemoteMirror library will use this if present to determine how to project the contents of the enum. If not present (for example, in older binaries), the RemoteMirror library falls back on an internal calculation of the spare bitmask.

Resolves rdar://61158214

@tbkka tbkka requested review from slavapestov and al45tair January 19, 2022 00:53
@tbkka tbkka force-pushed the tbkka-remoteMirror-MPE-spareBits-v2 branch from 59273d7 to 7663f5f Compare January 25, 2022 01:57
@tbkka tbkka force-pushed the tbkka-remoteMirror-MPE-spareBits-v2 branch from 7663f5f to 2210d22 Compare January 27, 2022 21:11
@tbkka
Copy link
Contributor Author

tbkka commented Feb 2, 2022

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2022

Build failed
Swift Test Linux Platform
Git Sha - 4693281f5d8764653df8d09056224478d5eab629

@swift-ci
Copy link
Contributor

swift-ci commented Feb 2, 2022

Build failed
Swift Test OS X Platform
Git Sha - 4693281f5d8764653df8d09056224478d5eab629

@tbkka tbkka force-pushed the tbkka-remoteMirror-MPE-spareBits-v2 branch from b0b49f2 to 3558747 Compare February 2, 2022 23:43
@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test

@tbkka tbkka requested a review from adrian-prantl February 3, 2022 00:06
@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test Windows platform

5 similar comments
@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 3, 2022

@swift-ci Please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 4, 2022

@swift-ci Please test Windows platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 4, 2022

@swift-ci Please test Linux platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 4, 2022

I added a bunch of std::cerr << and now the Windows failure is dumping this information:

reflection section too small!
Section Type: MultiPayloadEnum
Remaining section size: 24(of 28), size of next record: 261124
Previous bytes: 20  0 10  0 
Next bytes:  7  0  0  0  0  0  0 ff 

That information makes no sense. It's skipped one word so far, but MPE records are always at least two words. And the second word is 0x00000007 which is also nonsense, since the MPE record code can't generate that particular value for the second word.

My only guess is that this is not really a MultiPayloadEnum section at all. But staring at the source code isn't illuminating anything for me.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 5, 2022

@compnerd - Any thoughts on this? Another possibility that just occurred to me: Maybe on Windows, we get some extra data at the front of these sections? (This is probably the first reflection record type that's variably-sized, so it's almost conceivable that other reflection sections wouldn't be messed-up by extra bytes here.)

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - b5d31d0

@tbkka
Copy link
Contributor Author

tbkka commented Feb 10, 2022

@swift-ci Please test macOS Platform

@tbkka
Copy link
Contributor Author

tbkka commented Feb 10, 2022

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - fe070c8

@tbkka
Copy link
Contributor Author

tbkka commented Feb 10, 2022

@swift-ci Please test

@tbkka
Copy link
Contributor Author

tbkka commented Feb 11, 2022

@swift-ci Please test

@tbkka
Copy link
Contributor Author

tbkka commented Feb 11, 2022

@swift-ci Please test

@tbkka
Copy link
Contributor Author

tbkka commented Feb 11, 2022

@swift-ci Please test Windows

@tbkka
Copy link
Contributor Author

tbkka commented Feb 16, 2022

@swift-ci Please test

This means that if we don't have compiler information for a type that
has an enum in it somewhere, we'll refuse to project the enum value
because we don't know that we can do it correctly.

Also, disable the comparison of compiler-provided vs. locally-computed
masks for this case until this is fixed.
@tbkka
Copy link
Contributor Author

tbkka commented Feb 18, 2022

@swift-ci Please test

@tbkka
Copy link
Contributor Author

tbkka commented Feb 18, 2022

@swift-ci Please test

@tbkka
Copy link
Contributor Author

tbkka commented Feb 21, 2022

The definitions for all reflection sections are now part of llvm-project. Now that swiftlang/llvm-project#3966 has landed, this should build and run cleanly.

@tbkka
Copy link
Contributor Author

tbkka commented Feb 21, 2022

@swift-ci Please test

@tbkka tbkka merged commit 4d91b45 into swiftlang:main Feb 22, 2022
tbkka added a commit to tbkka/swift that referenced this pull request Jun 7, 2024
…ion metadata

This mostly reverts "[RemoteMirror] Get spare bit info from reflection records (swiftlang#40906)"
I recently figured out that RemoteMirror can compute all the spare
bit data directly without requiring this information.  So let's
remove it to save some space in binaries.

This mostly reverts commit 4d91b45.
tbkka added a commit to tbkka/swift that referenced this pull request Jun 13, 2024
without relying on spare bit information in the reflection metadata
(which was added in swiftlang#40906).  As a result, we can remove the
code from swiftlang#40906.

This is the first step in such removal.  It removes the RemoteMirror
code for looking up such metadata.  It leaves behind:

* Sufficient stubs for LLDB to continue to build.  Once LLDB is updated, these stubs can be removed as well.

* The compiler code to emit such metadata.  This allows new binaries to still reflect MPEs on older runtimes.  This will need to be kept for a transitional period.
meg-gupta pushed a commit to meg-gupta/swift that referenced this pull request Jun 20, 2024
without relying on spare bit information in the reflection metadata
(which was added in swiftlang#40906).  As a result, we can remove the
code from swiftlang#40906.

This is the first step in such removal.  It removes the RemoteMirror
code for looking up such metadata.  It leaves behind:

* Sufficient stubs for LLDB to continue to build.  Once LLDB is updated, these stubs can be removed as well.

* The compiler code to emit such metadata.  This allows new binaries to still reflect MPEs on older runtimes.  This will need to be kept for a transitional period.
@tbkka tbkka deleted the tbkka-remoteMirror-MPE-spareBits-v2 branch August 1, 2024 16:37
augusto2112 pushed a commit to augusto2112/swift that referenced this pull request Oct 16, 2024
without relying on spare bit information in the reflection metadata
(which was added in swiftlang#40906).  As a result, we can remove the
code from swiftlang#40906.

This is the first step in such removal.  It removes the RemoteMirror
code for looking up such metadata.  It leaves behind:

* Sufficient stubs for LLDB to continue to build.  Once LLDB is updated, these stubs can be removed as well.

* The compiler code to emit such metadata.  This allows new binaries to still reflect MPEs on older runtimes.  This will need to be kept for a transitional period.

(cherry picked from commit c20ef6d)
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.

6 participants