Skip to content

Add MCCAS support to SwiftCaching.cpp #76020

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 1 commit into from
Aug 28, 2024
Merged

Add MCCAS support to SwiftCaching.cpp #76020

merged 1 commit into from
Aug 28, 2024

Conversation

rastogishubham
Copy link
Contributor

@rastogishubham rastogishubham commented Aug 21, 2024 •
edited
Loading

To materialize the object file correctly on a cache hit when MCCAS is enabled, if lib_InternalSwiftScan.dylib is used to detect the cache hit, we need to add MCCAS support to the replay code in SwiftCaching.cpp

rdar://134426931 ([MCCAS] FIx bug with swift caching where object files are incorrect on a cache hit)

@rastogishubham rastogishubham force-pushed the FixReplay branch 3 times, most recently from 20f6afd to 618272e Compare August 21, 2024 18:12
@rastogishubham
Copy link
Contributor Author

@swift-ci please test

@rastogishubham
Copy link
Contributor Author

@swift-ci Please smoke test

@cachemeifyoucan
Copy link
Contributor

Change LGTM. I think we can add a test for that. You can access the replay logic inside libSwiftScan via swift-scan-test -action replay_result. For example, see: test/CAS/swift-scan-test.swift

@rastogishubham
Add MCCAS support to SwiftCaching.cpp
2458226 
To materialize the object file correctly on a cache hit when MCCAS is
enabled, if lib_InternalSwiftScan.dylib is used to detect the cache hit,
we need to add MCCAS support to the replay code in SwiftCaching.cpp
@rastogishubham
Copy link
Contributor Author

@swift-ci please smoke test

@rastogishubham
Copy link
Contributor Author

@swift-ci please test

cachemeifyoucan
cachemeifyoucan approved these changes Aug 22, 2024
View reviewed changes
Copy link
Contributor

@cachemeifyoucan cachemeifyoucan left a comment

Choose a reason for hiding this comment

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

Thanks!

@cachemeifyoucan
Copy link
Contributor 

Binary files /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/CAS/Output/swift-scan-test-mccas.swift.tmp/test.o and /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/CAS/Output/swift-scan-test-mccas.swift.tmp/test2.o differ

@rastogishubham
Copy link
Contributor Author 

Binary files /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/CAS/Output/swift-scan-test-mccas.swift.tmp/test.o and /Users/ec2-user/jenkins/workspace/swift-PR-macos-smoke-test/branch-main/build/buildbot_incremental/swift-macosx-x86_64/test-macosx-x86_64/CAS/Output/swift-scan-test-mccas.swift.tmp/test2.o differ

@cachemeifyoucan they differ right now because swiftlang/llvm-project#9164 needs to go in first, right now on a cache miss, the object files are 0 bytes in size.

@rastogishubham
Copy link
Contributor Author

Please test with following PR:
swiftlang/llvm-project#9164

@swift-ci Please test macOS platform

@rastogishubham
Copy link
Contributor Author

Please test with following PR:
swiftlang/llvm-project#9164

@swift-ci Please smoke test macOS platform

@rastogishubham
Copy link
Contributor Author

Please test with following PR:
swiftlang/llvm-project#9164

@swift-ci Please test macOS platform

@rastogishubham
Copy link
Contributor Author

Please test with following PR:
swiftlang/llvm-project#9164

@swift-ci Please smoke test macOS platform

@rastogishubham rastogishubham merged commit 6c1b3cc into swiftlang:main Aug 28, 2024
5 checks passed
@rastogishubham rastogishubham deleted the FixReplay branch August 28, 2024 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cachemeifyoucan cachemeifyoucan cachemeifyoucan approved these changes

@artemcm artemcm Awaiting requested review from artemcm

@tshortli tshortli Awaiting requested review from tshortli

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

Successfully merging this pull request may close these issues.
2 participants
@rastogishubham @cachemeifyoucan