Skip to content

Catch up with llvm/llvm-project@9114ac67a986400155b8b82013d09a9e4f48e534#1765

Merged
MrSidims merged 1 commit intoKhronosGroup:mainfrom
arichardson:annotation-intrinsics-llvm-16
Dec 8, 2022
Merged

Catch up with llvm/llvm-project@9114ac67a986400155b8b82013d09a9e4f48e534#1765
MrSidims merged 1 commit intoKhronosGroup:mainfrom
arichardson:annotation-intrinsics-llvm-16

Conversation

@arichardson
Copy link
Contributor

The ptr_annotation/var_annotation/annotation intrinsics gained additional overloads which means we have to update calls to Intrinsic::getDeclaration()/Builder.CreateIntrinsic().

@CLAassistant
Copy link

CLAassistant commented Dec 8, 2022

CLA assistant check
All committers have signed the CLA.

@MrSidims
Copy link
Contributor

MrSidims commented Dec 8, 2022

@arichardson thanks a lot! Just started looking at https://reviews.llvm.org/D138722

@arichardson
Copy link
Contributor Author

I believe a follow-up change would be to use the Constant address space for the global strings, but since I don't understand the required semantics here I did not include that in this PR.

Copy link
Contributor

@MrSidims MrSidims left a comment

Choose a reason for hiding this comment

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

LGTM
@arichardson please apply clang tidy suggestion

@arichardson arichardson force-pushed the annotation-intrinsics-llvm-16 branch from ac9a869 to db06f54 Compare December 8, 2022 10:50
@arichardson
Copy link
Contributor Author

LGTM @arichardson please apply clang tidy suggestion

Done. I guess the CI failures are caused by the builders using an older LLVM snapshot?

@MrSidims
Copy link
Contributor

MrSidims commented Dec 8, 2022

LGTM @arichardson please apply clang tidy suggestion

Done. I guess the CI failures are caused by the builders using an older LLVM snapshot?

Yeap, we can ignore them. in-tree builds and testing are more important in this case.

The ptr_annotation/var_annotation/annotation intrinsics gained
additional overloads which means we have to update calls to
Intrinsic::getDeclaration()/Builder.CreateIntrinsic().
@arichardson arichardson force-pushed the annotation-intrinsics-llvm-16 branch from db06f54 to a9f4fec Compare December 8, 2022 10:59
@arichardson
Copy link
Contributor Author

LGTM @arichardson please apply clang tidy suggestion

Done. I guess the CI failures are caused by the builders using an older LLVM snapshot?

Ah actually there were two clang-tidy warnings but it seems like only the first one showed up in the log. Fixed the second one now.

@MrSidims MrSidims merged commit 41d2056 into KhronosGroup:main Dec 8, 2022
@arichardson arichardson deleted the annotation-intrinsics-llvm-16 branch December 8, 2022 13:32
vmaksimo added a commit to vmaksimo/SPIRV-LLVM-Translator that referenced this pull request Dec 9, 2022
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.

3 participants