Skip to content
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

Reflect upstream const changes #105

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

tsymalla
Copy link
Contributor

@tsymalla tsymalla commented Oct 2, 2024

TableGenFnMain was changed to pass a const RecordKeeper &, so
reflect that change in llvm-dialects. Also, const Record * pointers
are handed out now.
Adds a bunch of changes to make Record * const overall.
Guard both versions with LLVM_MAIN_REVISION to ensure upstream and
downstream CI continues to work.

@tsymalla-AMD tsymalla-AMD marked this pull request as draft October 2, 2024 13:03
@tsymalla tsymalla force-pushed the recordkeeper_const branch from 4990dd2 to 0d1b1e1 Compare October 4, 2024 09:43
@tsymalla-AMD tsymalla-AMD changed the title Make Record * members const Reflect upstream const changes Oct 4, 2024
`TableGenFnMain` was changed to pass a `const RecordKeeper &`, so
reflect that change in `llvm-dialects`. Also, `const Record *` pointers
are handed out now.
Guard both versions with LLVM_MAIN_REVISION to ensure upstream and
downstream CI continues to work.
@tsymalla tsymalla force-pushed the recordkeeper_const branch from 0d1b1e1 to d0da7de Compare October 4, 2024 09:44
@tsymalla-AMD tsymalla-AMD marked this pull request as ready for review October 4, 2024 09:44
@tsymalla-AMD tsymalla-AMD requested a review from dstutt October 4, 2024 09:46
Copy link
Member

@dstutt dstutt left a comment

Choose a reason for hiding this comment

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

As we discussed - this seems the most pragmatic approach.
We should probably remove this again at some point when the change has propagated everywhere it is required.
LGTM

@jasilvanus
Copy link
Collaborator

Note we already have a dependency on LLVM_MAIN_REVISION elsewhere in llvm-dialects (although that one could likely be removed now).

@tsymalla-AMD
Copy link
Contributor

Note we already have a dependency on LLVM_MAIN_REVISION elsewhere in llvm-dialects (although that one could likely be removed now).

Yes, this single dependency can be removed now, I will take care of that later.

@tsymalla-AMD tsymalla-AMD merged commit c436594 into GPUOpen-Drivers:dev Oct 4, 2024
8 checks passed
@tsymalla
Copy link
Contributor Author

tsymalla commented Oct 7, 2024

Note we already have a dependency on LLVM_MAIN_REVISION elsewhere in llvm-dialects (although that one could likely be removed now).

Yes, this single dependency can be removed now, I will take care of that later.

#107

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.

4 participants