This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Let mangler accept GlobalDecl
ClosedPublic

Authored by yaxunl on Mar 5 2020, 10:29 AM.

Details

Diff Detail

Event Timeline

yaxunl created this revision.Mar 5 2020, 10:29 AM
rjmccall added inline comments.Mar 5 2020, 2:38 PM
clang/lib/CodeGen/CodeGenModule.cpp
1025–1026

What would be necessary in order for this to turn into just mangleName(GD, Out)? I suspect there are a lot of subtle assumptions between the different mangling methods today that could probably be broken down a bit and we'd end up with something much cleaner.

yaxunl marked an inline comment as done.Mar 6 2020, 8:03 AM
yaxunl added inline comments.
clang/lib/CodeGen/CodeGenModule.cpp
1025–1026

It seems to be just API changes. I will try changing them.

yaxunl marked an inline comment as done.Mar 6 2020, 4:10 PM
yaxunl added inline comments.
clang/lib/CodeGen/CodeGenModule.cpp
1025–1026

I got one issue. When I tried to create GlobalDecl(CD, Ctor_DefaultClosure) I got assertion

include/llvm/ADT/PointerIntPair.h:186: static intptr_t llvm::PointerIntPairInfo<const clang::Decl *, 2, llvm::PointerLikeTypeTraits<const clang::Decl *> >::updateInt(intptr_t, intptr_t) [PointerT = const clang::Decl *, IntBits = 2, PtrTraits = llvm::PointerLikeTypeTraits<const clang::Decl *>]: Assertion `(IntWord & ~IntMask) == 0 && "Integer too large for field"' failed.
CXXConstructor=v:7:25 (default constructor) (converting constructor)

This is because Ctor_DefaultClosure has value 4, which exceeds the value that can be put into PointerIntPairInfo

yaxunl updated this revision to Diff 248873.Mar 6 2020, 5:54 PM

Remove mangleCXXCtor/Dtor.

rjmccall added inline comments.Mar 6 2020, 7:12 PM
clang/lib/AST/Expr.cpp
659–660

This will need an extension for your case, right? Maybe there should be comments in GlobalDecl pointing out all the places where we need to go from an arbitrary Decl* to a GlobalDecl so that someone adding a new kind of discriminated declaration will know to update them all.

clang/lib/AST/ItaniumMangle.cpp
1563

cast? But I'm not sure this is true, local entities can be in non-function declarations: blocks, ObjC methods, and captured statements. You can just cast<Decl>(DC).

clang/lib/CodeGen/CodeGenModule.cpp
1025–1026

It looks like Decl is declared with alignment 8, so yeah, it's safe to make that a 3-bit field.

yaxunl updated this revision to Diff 248926.Mar 7 2020, 5:28 AM
yaxunl marked 6 inline comments as done.

revised by John's comments.

rjmccall accepted this revision.Mar 7 2020, 11:11 AM
rjmccall added inline comments.
clang/lib/AST/ItaniumMangle.cpp
1563

I guess we just never enter this for local names within ObjC methods or blocks?

This revision is now accepted and ready to land.Mar 7 2020, 11:11 AM
yaxunl marked 2 inline comments as done.Mar 7 2020, 12:57 PM
yaxunl added inline comments.
clang/lib/AST/Expr.cpp
659–660

Added extension for kernel reference here and other places in the other patch. Also added comments to GlobalDecl.

clang/lib/AST/ItaniumMangle.cpp
1563

Original code is cast<FunctionDecl>(DC) since ObjCMethodDecl and BlockDecl are handled separately. Since we dot have GlobalDecl ctor for Decl*, I will cast to FunctionDecl.

1563

Yes.

This revision was automatically updated to reflect the committed changes.
yaxunl marked an inline comment as done.
Herald added a project: Restricted Project. · View Herald TranscriptMar 7 2020, 9:00 PM