-
Notifications
You must be signed in to change notification settings - Fork 139
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
[feature][flang2] Initial support for opaque pointers #1321
[feature][flang2] Initial support for opaque pointers #1321
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks OK. The changes here overlap with #1322. What will be the policy of merging those?
@pawosm-arm #1321 and #1322 are stacked PRs, similar to stacked reviews on Phabricator. We can rebase #1322 once #1321 is merged, but they can be reviewed in parallel, although for #1322 you will have to select the top commit in order to isolate the changes. Also, we must decide at the call tomorrow whether we should create a new opaque-pointer master branch for LLVM 15 and newer. Below are some pros and cons I can think of. One master branch that handles both legacy IR and opaque pointer IR
Two branches
|
From the CI point of view, how big this problem is? |
Not a big problem at all. Let's assume that we create a branch named |
For me clearer code outweighs any infrastructural considerations. |
Should one option |
@PeixinQiao We have already decided at the biweekly technical call to not have an option to toggle the feature. If you don't want opaque pointers, use the |
@xinliu-hnc Could you rebase this on the master branch? |
Signed-off-by: Paul Osmialowski <[email protected]>
7f8a4bb
to
4eaaf77
Compare
OK. Thanks for the info. |
The branch is rebased. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This PR is based on PR #1306 submitted by @pawosm-arm, we have reviewed PR #1306 and made some changes on the original patch.
For typed pointers generated by functions, we modify the function in ll_structrue.cpp to uniformly use 'ptr' as the IR output string.
For some hard-coded typed pointers, we rewrite them with 'ptr', and remove some redundant cast_ops between pointers.
We also modify some test cases that use typed pointer IR.
In addition, this branch needs to be built with LLVM 15.