Skip to content

Enable usage of LLVM's opaque pointer #66077

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 10 commits into from
Jun 15, 2023
Merged

Enable usage of LLVM's opaque pointer #66077

merged 10 commits into from
Jun 15, 2023

Conversation

@aschwaighofer
Copy link
Contributor

@aschwaighofer aschwaighofer commented May 23, 2023

This PR enables usage of LLVM's opaque pointer type ptr during LLVM IR code generation (instead of usage of typed pointers e.g. i64 *).

I have added a %use_no_opaque_pointers substitution for tests which will expand to the clang frontend flag (-Xcc -Xclang -Xcc -no-opaque-pointers) which disables opaque pointers.

Example usage:

%target-swift-frontend %use_no_opaque_pointers -emit-ir %s | %FileCheck %s

This substitution can be used to run tests under the -no-opaque-pointers option.

All LLVM IR checking tests are moved to use this option to keep them passing. An invocation without the %FileCheck command was added to verify that the test does not crash.

-// RUN: %target-swift-frontend -parse-stdlib %s -emit-ir | %FileCheck %s
+// RUN: %target-swift-frontend %use_no_opaque_pointers -parse-stdlib %s -emit-ir | %FileCheck %s
+// RUN: %target-swift-frontend -parse-stdlib %s -emit-ir

This approach should facilitate incrementally moving the test to use opaque pointers.

rdar://108891584

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

@swift-ci test

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

We will need a newer version of Xcode, the version installed on the build has a libtool/ar binary that does not support opaque pointers in bitcode.

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

@swift-ci test linux

@felipepiovezan
Copy link
Contributor

felipepiovezan commented May 23, 2023

An invocation without the %FileCheck command was added to verify that the test does not crash.

Just to double check, as the tests are ported, should we also remove this line?

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

An invocation without the %FileCheck command was added to verify that the test does not crash.

Just to double check, as the tests are ported, should we also remove this line?

@felipepiovezan Yes please.

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

This PR will require Xcode 14.3 to pass all tests on macOS. This is due to the usage of bitcode files containing opaque pointers and tools in Xcode prior to 14.3 did not support opaque pointers.

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

@swift-ci test windows

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 23, 2023

@swift-ci test

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

1 similar comment
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from a5111db to b09a1d8 Compare May 24, 2023 17:16
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch 2 times, most recently from bac8fb0 to eebc6c8 Compare May 24, 2023 19:00
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from eebc6c8 to ffc4ffe Compare May 24, 2023 20:15
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from ffc4ffe to 3727381 Compare May 24, 2023 22:09
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test windows

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 24, 2023

@swift-ci test macos

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 25, 2023

@swift-ci test linux

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 25, 2023

@swift-ci test

2 similar comments
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 25, 2023

@swift-ci test

@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 26, 2023

@swift-ci test

@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from 56dde2d to 384054f Compare May 26, 2023 17:24
@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from 384054f to 1d3528c Compare May 26, 2023 17:27
@aschwaighofer aschwaighofer mentioned this pull request May 26, 2023
Draft
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented May 26, 2023

Please test with following PR:
swiftlang/llvm-project#6923
@swift-ci test

aschwaighofer added a commit to aschwaighofer/llvm-project that referenced this pull request Jun 8, 2023
@aschwaighofer
Disable two tests that fail after enabling opaque pointer types in Swift
d324445 
To be able to see how far we can get disable these two tests to see
whether other things break in CI.

swiftlang/swift#66077 is the PR that trys to enable
opaque pointers in Swift.

rdar://109831415
@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from 1d3528c to 3507409 Compare June 8, 2023 17:34
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented Jun 8, 2023

Please test with following PR:
swiftlang/llvm-project#6923
@swift-ci test

@aschwaighofer
Enable opaque pointers
654f21f
@aschwaighofer
Add a %use_no_opaque_pointers subsitution for tests
5d1378d 
This substitution can be used to run tests under the -no-opaque-pointers
option.
@aschwaighofer
Move tests over to use the %use_no_opaque_pointers option
c1a93e0
@aschwaighofer
Fix linux tests
316e982
@aschwaighofer
Fix LTO test to use just use libLTO.dylib
8f7e433 
This is necessary until the Xcode toolchain's libLTO.dylib supports opaque pointers
@aschwaighofer
Fix test/IRGen/objc_properties_ios.swift
307a6d8
@aschwaighofer
Try to only remove comdata if the global is a declaration
e6d070d
@aschwaighofer
Move windows tests to %use_no_opaque_pointers usage
d1d7438
@aschwaighofer
More IRGen test to move over
90c7193
@aschwaighofer
Attempt to fix debug info under opaque pointers
0d4dec1 
Based on swiftlang#66409

With the observation that the pre-opaque world was using bitcast as an
indication that the storage type and the type of the variable were
different. We can recover this information from the storage type of the
alloca and the storage type of the type info.
@aschwaighofer aschwaighofer force-pushed the wip_enable_opaque_pointers branch from 0af0b52 to 0d4dec1 Compare June 14, 2023 17:50
@aschwaighofer
Copy link
Contributor Author

aschwaighofer commented Jun 14, 2023

@swift-ci test

@aschwaighofer aschwaighofer merged commit 5d5dbd9 into swiftlang:main Jun 15, 2023
@felipepiovezan felipepiovezan mentioned this pull request Jun 16, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zoecarver zoecarver Awaiting requested review from zoecarver

@hyp hyp Awaiting requested review from hyp

@egorzhdan egorzhdan Awaiting requested review from egorzhdan egorzhdan is a code owner

@ktoso ktoso Awaiting requested review from ktoso ktoso is a code owner

@adrian-prantl adrian-prantl Awaiting requested review from adrian-prantl adrian-prantl is a code owner

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

@aschwaighofer @felipepiovezan