Skip to content

[clang][CodeGen] Make UnqualPtrTy truly unqualified #94388

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

[clang][CodeGen] Make UnqualPtrTy truly unqualified #94388

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cdf95a8
Unqualified `ptr`s should be truly unqualified, and not AS0 ptrs.
AlexVlx Jun 4, 2024
d05b71c
Merge branch 'main' of https://github.com/llvm/llvm-project into unqu…
AlexVlx Jun 4, 2024
b62f653
Merge branch 'main' of https://github.com/llvm/llvm-project into unqu…
AlexVlx Jun 5, 2024
894b48b
Merge branch 'main' of https://github.com/llvm/llvm-project into unqu…
AlexVlx Jun 6, 2024
5eef3a9
Pointer to default should work.
AlexVlx Jun 6, 2024
86c0ff9
Merge branch 'main' of https://github.com/llvm/llvm-project into unqu…
AlexVlx Jun 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ CodeGenModule::CodeGenModule(ASTContext &C,
IntTy = llvm::IntegerType::get(LLVMContext, C.getTargetInfo().getIntWidth());
IntPtrTy = llvm::IntegerType::get(LLVMContext,
C.getTargetInfo().getMaxPointerWidth());
UnqualPtrTy = llvm::PointerType::getUnqual(LLVMContext);
Int8PtrTy = llvm::PointerType::get(LLVMContext,
C.getTargetAddressSpace(LangAS::Default));
const llvm::DataLayout &DL = M.getDataLayout();
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/CodeGenTypeCache.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,11 @@ struct CodeGenTypeCache {
llvm::IntegerType *PtrDiffTy;
};

/// unqualified void*
llvm::PointerType *UnqualPtrTy;
Copy link
Member

Choose a reason for hiding this comment

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

I'd argue we should just remove this variable, and the few cases that actually want AS0 can call PointerType::get(0) directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not opposed to this in principle; might have some additional benefits in that it forces people to knowingly opt for something via PointerType::getUnqual / PointerType::get(some_as), rather than accidentally stumbling into it.


/// void*, void** in the target's default address space (often 0)
union {
llvm::PointerType *UnqualPtrTy;
llvm::PointerType *VoidPtrTy;
llvm::PointerType *Int8PtrTy;
llvm::PointerType *VoidPtrPtrTy;
Expand Down
6 changes: 3 additions & 3 deletions clang/lib/CodeGen/ItaniumCXXABI.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4656,14 +4656,14 @@ static void InitCatchParam(CodeGenFunction &CGF,
auto catchRD = CatchType->getAsCXXRecordDecl();
CharUnits caughtExnAlignment = CGF.CGM.getClassPointerAlignment(catchRD);

llvm::Type *PtrTy = CGF.UnqualPtrTy; // addrspace 0 ok
Copy link
Member

Choose a reason for hiding this comment

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

I am not convinced this is actually correct, it's been a long time since I looked at C++ exception handling, but I would have assumed this to either be a stack variable or a global?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be a pointer to a heap-allocated object.

In any case, we shouldn't need to cast the return value of __cxa_begin_catch to a different address-space; we can just drop the bitcasts.

llvm::Type *PtrTy = CGF.UnqualPtrTy; // unqualified is ok

// Check for a copy expression. If we don't have a copy expression,
// that means a trivial copy is okay.
const Expr *copyExpr = CatchParam.getInit();
if (!copyExpr) {
llvm::Value *rawAdjustedExn = CallBeginCatch(CGF, Exn, true);
Address adjustedExn(CGF.Builder.CreateBitCast(rawAdjustedExn, PtrTy),
Address adjustedExn(CGF.Builder.CreatePointerCast(rawAdjustedExn, PtrTy),
LLVMCatchTy, caughtExnAlignment);
LValue Dest = CGF.MakeAddrLValue(ParamAddr, CatchType);
LValue Src = CGF.MakeAddrLValue(adjustedExn, CatchType);
Expand All @@ -4677,7 +4677,7 @@ static void InitCatchParam(CodeGenFunction &CGF,
CGF.EmitNounwindRuntimeCall(getGetExceptionPtrFn(CGF.CGM), Exn);

// Cast that to the appropriate type.
Address adjustedExn(CGF.Builder.CreateBitCast(rawAdjustedExn, PtrTy),
Address adjustedExn(CGF.Builder.CreatePointerCast(rawAdjustedExn, PtrTy),
LLVMCatchTy, caughtExnAlignment);

// The copy expression is defined in terms of an OpaqueValueExpr.
Expand Down