Skip to content

IRGen: Use the header size for the initial offset in non fixed heap l… #31049

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 16, 2020
Merged

IRGen: Use the header size for the initial offset in non fixed heap l… #31049

merged 2 commits into from
Apr 16, 2020

Conversation

aschwaighofer
Copy link
Contributor

…ayouts

When the first element in the heap layout was non fixed we would use the
mininum size of the total heap layout for the initial offset. This would
create unneccessary large heap layouts.

rdar://61716736

@aschwaighofer
IRGen: Use the header size for the initial offset in non fixed heap l…
53e9064 
…ayouts

When the first element in the heap layout was non fixed we would use the
mininum size of the total heap layout for the initial offset. This would
create unneccessary large heap layouts.

rdar://61716736
@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 53e9064

@aschwaighofer
Copy link
Contributor Author

The source compat failure is unrelated:

/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.4.sdk/usr/include/stdlib.h:61:10: fatal error: cannot open file '/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneOS.platform/Developer/SDKs/iPhoneOS13.4.sdk/usr/include/Availability.h': Operation not permitted #include <Availability.h> ^ 1 error generated.

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 53e9064

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 53e9064

@aschwaighofer
Copy link
Contributor Author

@swift-ci Please test source compat

slavapestov
slavapestov reviewed Apr 16, 2020
View reviewed changes
Copy link
Contributor

@slavapestov slavapestov left a comment

Choose a reason for hiding this comment

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

I think it's unfortunate that we support partial_apply contexts with non-fixed offsets. Did we give up on using SILBoxType for this purpose?

lib/IRGen/GenHeap.cpp
@@ -285,7 +285,7 @@ static llvm::Value *calcInitOffset(swift::irgen::IRGenFunction &IGF,
const swift::irgen::HeapLayout &layout) {
llvm::Value *offset = nullptr;
if (i == 0) {
auto startoffset = layout.getSize();
auto startoffset = layout.getHeaderSize();
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you rename this to startOffset?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change in a follow-up.

@aschwaighofer
Copy link
Contributor Author

The source compatibility failure is unrelated:

SIL memory lifetime failure in @$s10Kingfisher3leeoiySbAA0A15OptionsInfoItemO_ADtF: memory is initialized, but shouldn't
memory location:   %76 = alloc_stack $KingfisherOptionsInfoItem    // users: %87, %81, %77
at instruction:   dealloc_stack %76 : $*KingfisherOptionsInfoItem // id: %81

@aschwaighofer aschwaighofer merged commit 9599de2 into swiftlang:master Apr 16, 2020
@aschwaighofer
Copy link
Contributor Author

I don't think we have given up on using SILBoxType's for the representation of closures. 'Just' needs some work in SILGen, SIL, IRGen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slavapestov slavapestov slavapestov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aschwaighofer @swift-ci @slavapestov