[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.
tahonermann created this revision.
tahonermann added reviewers: erichkeane, aaron.ballman.
Herald added a project: All.
tahonermann added inline comments.
tahonermann published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3771
if (FD->isMultiVersion()) {
-UpdateMultiVersionNames(GD, FD, MangledName);
if (!IsForDefinition)
This is a drive-by indentation fix.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D122955
Files:
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -348,8 +348,9 @@
/// is defined once we get to the end of the of the translation unit.
std::vector Aliases;
- /// List of multiversion functions that have to be emitted. Used to make
sure
- /// we properly emit the iFunc.
+ /// List of multiversion functions to be emitted. This list is processed in
+ /// conjunction with other deferred symbols and is used to ensure that
+ /// multiversion function resolvers and ifuncs are defined and emitted.
std::vector MultiVersionFuncs;
typedef llvm::StringMap > ReplacementsTy;
@@ -1466,9 +1467,20 @@
llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
ForDefinition_t IsForDefinition = NotForDefinition);
+ // References to multiversion functions are resolved through an implicitly
+ // defined resolver function. This function is responsible for creating
+ // the resolver symbol for the provided declaration. The value returned
+ // will be for an ifunc (llvm::GlobalIFunc) if the current target supports
+ // that feature and for a regular function (llvm::GlobalValue) otherwise.
llvm::Constant *GetOrCreateMultiVersionResolver(GlobalDecl GD,
llvm::Type *DeclTy,
const FunctionDecl *FD);
+
+ // In scenarios where a function is not known to be a multiversion function
+ // until a later declaration, it is sometimes necessary to change the
+ // previously created mangled name to align with requirements of whatever
+ // multiversion function kind the function is now known to be. This function
+ // is responsible for performing such mangled name updates.
void UpdateMultiVersionNames(GlobalDecl GD, const FunctionDecl *FD,
StringRef &CurName);
@@ -1561,6 +1573,7 @@
// registered by the atexit subroutine using unatexit.
void unregisterGlobalDtorsWithUnAtExit();
+ /// Emit deferred multiversion function resolvers and associated variants.
void emitMultiVersionFunctions();
/// Emit any vtables which we deferred and still have a use for.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3768,7 +3768,7 @@
}
if (FD->isMultiVersion()) {
-UpdateMultiVersionNames(GD, FD, MangledName);
+ UpdateMultiVersionNames(GD, FD, MangledName);
if (!IsForDefinition)
return GetOrCreateMultiVersionResolver(GD, Ty, FD);
}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2723,8 +2723,13 @@
StringRef getFeatureStr(unsigned Index) const {
return *(featuresStrs_begin() + Index);
}
-// 'default' is always moved to the end, so it isn't considered
-// when mangling the index.
+// Given an index into the 'featuresStrs' sequence, compute a unique
+// ID to be used with function name mangling for the associated variant.
+// This mapping is necessary due to a requirement that the mangling ID
+// used for the "default" variant be the largest mangling ID in the
+// variant set. Note that, if duplicate variants are present in
+// 'featuresStrs', that each instance will be assigned its own unique
+// ID (the mapping is bijective).
unsigned getMangledIndex(unsigned Index) const {
if (getFeatureStr(Index) == "default")
return std::count_if(featuresStrs_begin(), featuresStrs_end(),
@@ -2734,9 +2739,10 @@
[](StringRef S) { return S != "default"; });
}
-// True if this is the first of this version to appear in the config
string.
-// This is used to make sure we don't try to emit this function multiple
-// times.
+// Given an index into the 'featuresStrs' sequence, determine if the
+// index corresponds to the first instance of the named variant. This
+// is used to skip over duplicate
[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.
erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Basic/Attr.td:2730 +// used for the "default" variant be the largest mangling ID in the +// variant set. Note that, if duplicate variants are present in +// 'featuresStrs', that each instance will be assigned its own unique this comma doesn't read right to me, I don't think it is required. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122955/new/ https://reviews.llvm.org/D122955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.
tahonermann updated this revision to Diff 420586.
tahonermann added a comment.
Reworded a comment to address code review commentary.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122955/new/
https://reviews.llvm.org/D122955
Files:
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -348,8 +348,9 @@
/// is defined once we get to the end of the of the translation unit.
std::vector Aliases;
- /// List of multiversion functions that have to be emitted. Used to make
sure
- /// we properly emit the iFunc.
+ /// List of multiversion functions to be emitted. This list is processed in
+ /// conjunction with other deferred symbols and is used to ensure that
+ /// multiversion function resolvers and ifuncs are defined and emitted.
std::vector MultiVersionFuncs;
typedef llvm::StringMap > ReplacementsTy;
@@ -1466,9 +1467,20 @@
llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
ForDefinition_t IsForDefinition = NotForDefinition);
+ // References to multiversion functions are resolved through an implicitly
+ // defined resolver function. This function is responsible for creating
+ // the resolver symbol for the provided declaration. The value returned
+ // will be for an ifunc (llvm::GlobalIFunc) if the current target supports
+ // that feature and for a regular function (llvm::GlobalValue) otherwise.
llvm::Constant *GetOrCreateMultiVersionResolver(GlobalDecl GD,
llvm::Type *DeclTy,
const FunctionDecl *FD);
+
+ // In scenarios where a function is not known to be a multiversion function
+ // until a later declaration, it is sometimes necessary to change the
+ // previously created mangled name to align with requirements of whatever
+ // multiversion function kind the function is now known to be. This function
+ // is responsible for performing such mangled name updates.
void UpdateMultiVersionNames(GlobalDecl GD, const FunctionDecl *FD,
StringRef &CurName);
@@ -1561,6 +1573,7 @@
// registered by the atexit subroutine using unatexit.
void unregisterGlobalDtorsWithUnAtExit();
+ /// Emit deferred multiversion function resolvers and associated variants.
void emitMultiVersionFunctions();
/// Emit any vtables which we deferred and still have a use for.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3768,7 +3768,7 @@
}
if (FD->isMultiVersion()) {
-UpdateMultiVersionNames(GD, FD, MangledName);
+ UpdateMultiVersionNames(GD, FD, MangledName);
if (!IsForDefinition)
return GetOrCreateMultiVersionResolver(GD, Ty, FD);
}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2723,8 +2723,12 @@
StringRef getFeatureStr(unsigned Index) const {
return *(featuresStrs_begin() + Index);
}
-// 'default' is always moved to the end, so it isn't considered
-// when mangling the index.
+// Given an index into the 'featuresStrs' sequence, compute a unique
+// ID to be used with function name mangling for the associated variant.
+// This mapping is necessary due to a requirement that the mangling ID
+// used for the "default" variant be the largest mangling ID in the
+// variant set. Duplicate variants present in 'featuresStrs' are also
+// assigned their own unique ID (the mapping is bijective).
unsigned getMangledIndex(unsigned Index) const {
if (getFeatureStr(Index) == "default")
return std::count_if(featuresStrs_begin(), featuresStrs_end(),
@@ -2734,9 +2738,10 @@
[](StringRef S) { return S != "default"; });
}
-// True if this is the first of this version to appear in the config
string.
-// This is used to make sure we don't try to emit this function multiple
-// times.
+// Given an index into the 'featuresStrs' sequence, determine if the
+// index corresponds to the first instance of the named variant. This
+// is used to skip over duplicate variant instances when iterating over
+// 'featuresStrs'.
bool isFirstOfVersion(unsigned Index) const {
StringRef FeatureStr(getFeatureStr(Index));
return 0 == std::count_if(
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/l
[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.
erichkeane accepted this revision. erichkeane added a comment. Still LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122955/new/ https://reviews.llvm.org/D122955 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
tahonermann marked an inline comment as done.
Closed by commit rGbed5ee3f4ba2: [clang] NFC: Enhance comments in CodeGen for
multiversion function support. (authored by tahonermann).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122955/new/
https://reviews.llvm.org/D122955
Files:
clang/include/clang/Basic/Attr.td
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/CodeGen/CodeGenModule.h
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -348,8 +348,9 @@
/// is defined once we get to the end of the of the translation unit.
std::vector Aliases;
- /// List of multiversion functions that have to be emitted. Used to make
sure
- /// we properly emit the iFunc.
+ /// List of multiversion functions to be emitted. This list is processed in
+ /// conjunction with other deferred symbols and is used to ensure that
+ /// multiversion function resolvers and ifuncs are defined and emitted.
std::vector MultiVersionFuncs;
typedef llvm::StringMap > ReplacementsTy;
@@ -1466,9 +1467,20 @@
llvm::AttributeList ExtraAttrs = llvm::AttributeList(),
ForDefinition_t IsForDefinition = NotForDefinition);
+ // References to multiversion functions are resolved through an implicitly
+ // defined resolver function. This function is responsible for creating
+ // the resolver symbol for the provided declaration. The value returned
+ // will be for an ifunc (llvm::GlobalIFunc) if the current target supports
+ // that feature and for a regular function (llvm::GlobalValue) otherwise.
llvm::Constant *GetOrCreateMultiVersionResolver(GlobalDecl GD,
llvm::Type *DeclTy,
const FunctionDecl *FD);
+
+ // In scenarios where a function is not known to be a multiversion function
+ // until a later declaration, it is sometimes necessary to change the
+ // previously created mangled name to align with requirements of whatever
+ // multiversion function kind the function is now known to be. This function
+ // is responsible for performing such mangled name updates.
void UpdateMultiVersionNames(GlobalDecl GD, const FunctionDecl *FD,
StringRef &CurName);
@@ -1561,6 +1573,7 @@
// registered by the atexit subroutine using unatexit.
void unregisterGlobalDtorsWithUnAtExit();
+ /// Emit deferred multiversion function resolvers and associated variants.
void emitMultiVersionFunctions();
/// Emit any vtables which we deferred and still have a use for.
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3768,7 +3768,7 @@
}
if (FD->isMultiVersion()) {
-UpdateMultiVersionNames(GD, FD, MangledName);
+ UpdateMultiVersionNames(GD, FD, MangledName);
if (!IsForDefinition)
return GetOrCreateMultiVersionResolver(GD, Ty, FD);
}
Index: clang/include/clang/Basic/Attr.td
===
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -2723,8 +2723,12 @@
StringRef getFeatureStr(unsigned Index) const {
return *(featuresStrs_begin() + Index);
}
-// 'default' is always moved to the end, so it isn't considered
-// when mangling the index.
+// Given an index into the 'featuresStrs' sequence, compute a unique
+// ID to be used with function name mangling for the associated variant.
+// This mapping is necessary due to a requirement that the mangling ID
+// used for the "default" variant be the largest mangling ID in the
+// variant set. Duplicate variants present in 'featuresStrs' are also
+// assigned their own unique ID (the mapping is bijective).
unsigned getMangledIndex(unsigned Index) const {
if (getFeatureStr(Index) == "default")
return std::count_if(featuresStrs_begin(), featuresStrs_end(),
@@ -2734,9 +2738,10 @@
[](StringRef S) { return S != "default"; });
}
-// True if this is the first of this version to appear in the config
string.
-// This is used to make sure we don't try to emit this function multiple
-// times.
+// Given an index into the 'featuresStrs' sequence, determine if the
+// index corresponds to the first instance of the named variant. This
+// is used to skip over duplicate variant instances when iterating over
+// 'featuresStrs'.
bool isFirstOfVersion(unsigned Index) const {
StringRef FeatureStr(getFeatureStr(Index));
return 0
