[clang][NFC] Adjust TBAA Base Info API #73263
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo. 1) isValidBaseType explicitly checks for a reference type to return false, but then also returns false for all non-record types. Just remove that reference check. 2) All uses of CodeGenTBAA::getBaseTypeInfo from within that class are when we've already checked the type isValidBaseType. The only case where this isn't true is from outside the class. It seems better to have two entry points in this case. Adding a new 'maybeGetBaseTypeInfo' entry point for external uses that returns nullptr for non valid base types. (Open to other names?) [This is part one of a pair of changes.]
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Nathan Sidwell (urnathan) ChangesI noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
[This is part one of a pair of changes.] Full diff: https://github.com/llvm/llvm-project/pull/73263.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 7cdf50a281cd278..e01fdc3579d88b8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1318,7 +1318,7 @@ llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType QTy) {
llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
if (!TBAA)
return nullptr;
- return TBAA->getBaseTypeInfo(QTy);
+ return TBAA->maybeGetBaseTypeInfo(QTy);
}
llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..9b45a644937b8d9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
- return false;
if (const RecordType *TTy = QTy->getAs<RecordType>()) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
// Incomplete types are not valid base access types.
@@ -414,8 +412,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) {
}
llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
- if (!isValidBaseType(QTy))
- return nullptr;
+ assert(isValidBaseType(QTy) && "Must be a valid base type");
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
@@ -428,6 +425,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return BaseTypeMetadataCache[Ty] = TypeNode;
}
+llvm::MDNode *CodeGenTBAA::maybeGetBaseTypeInfo(QualType QTy) {
+ return isValidBaseType(QTy) ? getBaseTypeInfo(QTy) : nullptr;
+}
+
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
assert(!Info.isIncomplete() && "Access to an object of an incomplete type!");
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index a65963596fe9def..53d77e1fefc4352 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -166,6 +166,10 @@ class CodeGenTBAA {
/// used to describe accesses to objects of the given base type.
llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);
+ /// getBaseTypeInfo - Return metadata that describes the given base access
+ /// type. The type must be suitable.
+ llvm::MDNode *getBaseTypeInfo(QualType QTy);
+
public:
CodeGenTBAA(ASTContext &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext);
@@ -187,9 +191,10 @@ class CodeGenTBAA {
/// the given type.
llvm::MDNode *getTBAAStructInfo(QualType QTy);
- /// getBaseTypeInfo - Get metadata that describes the given base access type.
- /// Return null if the type is not suitable for use in TBAA access tags.
- llvm::MDNode *getBaseTypeInfo(QualType QTy);
+ /// maybeGetBaseTypeInfo - Get metadata that describes the given base access
+ /// type. Return null if the type is not suitable for use in TBAA access
+ /// tags.
+ llvm::MDNode *maybeGetBaseTypeInfo(QualType QTy);
/// getAccessTagInfo - Get TBAA tag for a given memory access.
llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
|
|
@llvm/pr-subscribers-clang-codegen Author: Nathan Sidwell (urnathan) ChangesI noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
[This is part one of a pair of changes.] Full diff: https://github.com/llvm/llvm-project/pull/73263.diff 3 Files Affected:
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 7cdf50a281cd278..e01fdc3579d88b8 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1318,7 +1318,7 @@ llvm::MDNode *CodeGenModule::getTBAAStructInfo(QualType QTy) {
llvm::MDNode *CodeGenModule::getTBAABaseTypeInfo(QualType QTy) {
if (!TBAA)
return nullptr;
- return TBAA->getBaseTypeInfo(QTy);
+ return TBAA->maybeGetBaseTypeInfo(QTy);
}
llvm::MDNode *CodeGenModule::getTBAAAccessTagInfo(TBAAAccessInfo Info) {
diff --git a/clang/lib/CodeGen/CodeGenTBAA.cpp b/clang/lib/CodeGen/CodeGenTBAA.cpp
index 8705d3d65f1a573..9b45a644937b8d9 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.cpp
+++ b/clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -95,8 +95,6 @@ static bool TypeHasMayAlias(QualType QTy) {
/// Check if the given type is a valid base type to be used in access tags.
static bool isValidBaseType(QualType QTy) {
- if (QTy->isReferenceType())
- return false;
if (const RecordType *TTy = QTy->getAs<RecordType>()) {
const RecordDecl *RD = TTy->getDecl()->getDefinition();
// Incomplete types are not valid base access types.
@@ -414,8 +412,7 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfoHelper(const Type *Ty) {
}
llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
- if (!isValidBaseType(QTy))
- return nullptr;
+ assert(isValidBaseType(QTy) && "Must be a valid base type");
const Type *Ty = Context.getCanonicalType(QTy).getTypePtr();
if (llvm::MDNode *N = BaseTypeMetadataCache[Ty])
@@ -428,6 +425,10 @@ llvm::MDNode *CodeGenTBAA::getBaseTypeInfo(QualType QTy) {
return BaseTypeMetadataCache[Ty] = TypeNode;
}
+llvm::MDNode *CodeGenTBAA::maybeGetBaseTypeInfo(QualType QTy) {
+ return isValidBaseType(QTy) ? getBaseTypeInfo(QTy) : nullptr;
+}
+
llvm::MDNode *CodeGenTBAA::getAccessTagInfo(TBAAAccessInfo Info) {
assert(!Info.isIncomplete() && "Access to an object of an incomplete type!");
diff --git a/clang/lib/CodeGen/CodeGenTBAA.h b/clang/lib/CodeGen/CodeGenTBAA.h
index a65963596fe9def..53d77e1fefc4352 100644
--- a/clang/lib/CodeGen/CodeGenTBAA.h
+++ b/clang/lib/CodeGen/CodeGenTBAA.h
@@ -166,6 +166,10 @@ class CodeGenTBAA {
/// used to describe accesses to objects of the given base type.
llvm::MDNode *getBaseTypeInfoHelper(const Type *Ty);
+ /// getBaseTypeInfo - Return metadata that describes the given base access
+ /// type. The type must be suitable.
+ llvm::MDNode *getBaseTypeInfo(QualType QTy);
+
public:
CodeGenTBAA(ASTContext &Ctx, llvm::Module &M, const CodeGenOptions &CGO,
const LangOptions &Features, MangleContext &MContext);
@@ -187,9 +191,10 @@ class CodeGenTBAA {
/// the given type.
llvm::MDNode *getTBAAStructInfo(QualType QTy);
- /// getBaseTypeInfo - Get metadata that describes the given base access type.
- /// Return null if the type is not suitable for use in TBAA access tags.
- llvm::MDNode *getBaseTypeInfo(QualType QTy);
+ /// maybeGetBaseTypeInfo - Get metadata that describes the given base access
+ /// type. Return null if the type is not suitable for use in TBAA access
+ /// tags.
+ llvm::MDNode *maybeGetBaseTypeInfo(QualType QTy);
/// getAccessTagInfo - Get TBAA tag for a given memory access.
llvm::MDNode *getAccessTagInfo(TBAAAccessInfo Info);
|
|
sure, like so? |
This naming scheme better? |
|
ping? |
|
@efriedma-quic this naming ok? |
AaronBallman
reviewed
Jan 8, 2024
AaronBallman
approved these changes
Apr 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I noticed a couple of minor issues with CodeGenTBAA::getBaseTypeInfo.
isValidBaseType explicitly checks for a reference type to return false, but then also returns false for all non-record types. Just remove that reference check.
All uses of CodeGenTBAA::getBaseTypeInfo from within that class are when we've already checked the type isValidBaseType. The only case where this isn't true is from outside the class. It seems better to have two entry points in this case. Adding a new
'maybeGetBaseTypeInfo' entry point for external uses that returns nullptr for non valid base types. (Open to other names?)
[This is part one of a pair of changes.]