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

[CIR][CIRGen] virtual table pointer initialization without ctor #1283

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

ghehg
Copy link
Collaborator

@ghehg ghehg commented Jan 14, 2025

Corresponding OG code.
OG generated code here, one notable diff is we're missing inrange which is reported in issue 886 .
For now, I'm still using GlobalViewAttr to implement it so we can move things fast.
But it might be worth considering approach Comments in issue 258, especially we could incoporate inrange info to the attribute suggested there.

@ghehg ghehg marked this pull request as ready for review January 14, 2025 18:17
@@ -468,6 +468,9 @@ struct MissingFeatures {
static bool mustProgress() { return false; }

static bool skipTempCopy() { return false; }

// https://clang.llvm.org/docs/PointerAuthentication.html
static bool pointerAuthentication() { return false; }
Copy link
Member

Choose a reason for hiding this comment

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

We already have ptrAuth, use that

@bcardosolopes bcardosolopes merged commit 1029b19 into llvm:main Jan 14, 2025
6 checks passed
lanza pushed a commit that referenced this pull request Mar 18, 2025
Corresponding [OG
code](https://github.com/llvm/clangir/blob/ef20d053b3d78c9d4c135e2811b303b7e5016d30/clang/lib/CodeGen/CGExprConstant.cpp#L846).
[OG generated code here](https://godbolt.org/z/x6q333dMn), one notable
diff is we're missing `inrange` which is reported in [issue 886
](#886).
For now, I'm still using GlobalViewAttr to implement it so we can move
things fast.
But it might be worth considering approach [Comments in issue
258](#258), especially we could
incoporate [inrange info](#886) to
the attribute suggested there.
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.

2 participants