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

llvm.var.annotation and llvm.ptr.annotation aren't auto-upgraded #48350

Closed
andykaylor opened this issue Feb 3, 2021 · 10 comments
Closed

llvm.var.annotation and llvm.ptr.annotation aren't auto-upgraded #48350

andykaylor opened this issue Feb 3, 2021 · 10 comments
Labels
bugzilla Issues migrated from bugzilla

Comments

@andykaylor
Copy link
Contributor

Bugzilla Link 49006
Resolution FIXED
Resolved on Feb 23, 2021 14:07
Version unspecified
OS All
Blocks #48246 #48661
CC @tstellar,@Ralender
Fixed by commit(s) 9a82790 eccac5a

Extended Description

This commit (https://reviews.llvm.org/D88645) added additional arguments to the llvm.var.annotation and llvm.ptr.annotation intrinsics, but those intrinsics are not autograded when read from old bitcode files.

The Language Reference description of these intrinsics is also out of date.

@tstellar
Copy link
Collaborator

The patch was committed as d3205bb, but it looks like there were some follow-ups. Can someone post the full lits of commits that need to be backported.

@andykaylor
Copy link
Contributor Author

I believe d3205bb was the commit that added the extra arguments to the intrinsics. Some tests were fixed in 4afa077 and an example was updated in 2618247, but those changes don't affect the bitcode.

Active patches that need to land and be put into the 12.0 branch are:

https://reviews.llvm.org/D96646 -- Updates the LangRef
https://reviews.llvm.org/D95993 -- Auto-upgrade the intrinsics in bitcode

The initial commit also added an extra field to the structure used to initialize @​llvm.global.annotations. There may be some feature degradation with that change, but the feature seems to still be actively evolving, and this change doesn't prevent older bitcode files from being read or cause incorrect compilation.

@tstellar
Copy link
Collaborator

OK, once those last 2 patches are pushed to main, I will backport the whole set.

@tstellar
Copy link
Collaborator

These fixes are at risk of missing the 12.0.0 release, because there are still patches outstanding. What are the consequences if we don't get these into the 12.0.0 release?

@tstellar
Copy link
Collaborator

This bug was not resolved in time for the 12.0.0 release, so it will have to wait for 12.0.1.

If you feel this is a high-priority bug that should be fixed for 12.0.0, please re-add release-12.0.0 to the Blocks field and add a comment explaining why this is high-priority.

@andykaylor
Copy link
Contributor Author

The consequence of not getting https://reviews.llvm.org/D95993 into the 12.0 release is that any 11.x bitcode file containing the annotation intrinsics will not be be readable by a 12.0 tool. I don't know how commonly used these intrinsics are. This should at least get a release note.

I think the patch is very low risk, but the list of reviewers include the people who have been active in implementing the intrinsic, so if they don't consider this problem urgent for the 12.0 release perhaps a release note is sufficient.

https://reviews.llvm.org/D96646 is a documentation only change, but if it doesn't land, the documentation will be incorrect. Users can't fail to discover that if they try to make use of the intrinsics with the incorrectly documented number of parameters.

@tstellar
Copy link
Collaborator

Let's give this one more day to see if any progress can be made.

@andykaylor
Copy link
Contributor Author

The auto-upgrade change has landed: 9a82790

@tstellar
Copy link
Collaborator

Merged: eccac5a

@tstellar
Copy link
Collaborator

mentioned in issue #48661

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 11, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla
Projects
None yet
Development

No branches or pull requests

2 participants