[CIR] Infrastructure: class CIRGenBuilderTy; cache CIR types #119037
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
Small infrastructure and background changes to ClangIR. Create class `CIRGenBuilderTy` and its base class `CIRBaseBuilderTy`. These are mostly empty for now, except for what is inherited from `mlir::OpBuilder`. But they will fill up quickly as more ClangIR code gen is upstreamed. `CIRGenModule` and `CIRGenTypes` are changed to use `CIRGenBuilderTy`. Add cached types to struct `CIRGenTypeCache` for the integral types that are currently supported. Initialize those cached types in the `CIRGenModule` constructor. The first uses of those types (well, one of them) is in `CIRGenTypes::convertType`. Have `CIRGenTypes::convertType` cache its results in a map from `clang::Type` to `mlir::Type`, saving it from having to convert the same type again and again. There are no new tests or changed tests in this commit. There are no changes to behavior, just improvements to how the existing behavior is implemented.
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: David Olsen (dkolsen-pgi) ChangesSmall infrastructure and background changes to ClangIR. Create class Add cached types to struct Have There are no new tests or changed tests in this commit. There are no changes to behavior, just improvements to how the existing behavior is implemented. Full diff: https://github.com/llvm/llvm-project/pull/119037.diff 7 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
new file mode 100644
index 00000000000000..08281032dc2672
--- /dev/null
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -0,0 +1,25 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_CIRBASEBUILDER_H
+#define LLVM_CLANG_LIB_CIRBASEBUILDER_H
+
+#include "mlir/IR/Builders.h"
+
+namespace cir {
+
+class CIRBaseBuilderTy : public mlir::OpBuilder {
+
+public:
+ CIRBaseBuilderTy(mlir::MLIRContext &mlirContext)
+ : mlir::OpBuilder(&mlirContext) {}
+};
+
+} // namespace cir
+
+#endif
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
new file mode 100644
index 00000000000000..f16f25d8bfb0b6
--- /dev/null
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -0,0 +1,28 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CIRGENBUILDER_H
+#define LLVM_CLANG_LIB_CIR_CIRGENBUILDER_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/CIR/Dialect/Builder/CIRBaseBuilder.h"
+
+namespace clang::CIRGen {
+
+class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
+ const CIRGenTypeCache &typeCache;
+
+public:
+ CIRGenBuilderTy(mlir::MLIRContext &mlirContext, const CIRGenTypeCache &tc)
+ : CIRBaseBuilderTy(mlirContext), typeCache(tc) {}
+};
+
+} // namespace clang::CIRGen
+
+#endif
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index b44f66493254f2..e7c9512dcd3de8 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -29,9 +29,22 @@ CIRGenModule::CIRGenModule(mlir::MLIRContext &context,
clang::ASTContext &astctx,
const clang::CodeGenOptions &cgo,
DiagnosticsEngine &diags)
- : builder(&context), astCtx(astctx), langOpts(astctx.getLangOpts()),
+ : builder(context, *this), astCtx(astctx), langOpts(astctx.getLangOpts()),
theModule{mlir::ModuleOp::create(mlir::UnknownLoc::get(&context))},
- diags(diags), target(astCtx.getTargetInfo()), genTypes(*this) {}
+ diags(diags), target(astctx.getTargetInfo()), genTypes(*this) {
+
+ // Initialize cached types
+ SInt8Ty = cir::IntType::get(&getMLIRContext(), 8, /*isSigned=*/true);
+ SInt16Ty = cir::IntType::get(&getMLIRContext(), 16, /*isSigned=*/true);
+ SInt32Ty = cir::IntType::get(&getMLIRContext(), 32, /*isSigned=*/true);
+ SInt64Ty = cir::IntType::get(&getMLIRContext(), 64, /*isSigned=*/true);
+ SInt128Ty = cir::IntType::get(&getMLIRContext(), 128, /*isSigned=*/true);
+ UInt8Ty = cir::IntType::get(&getMLIRContext(), 8, /*isSigned=*/false);
+ UInt16Ty = cir::IntType::get(&getMLIRContext(), 16, /*isSigned=*/false);
+ UInt32Ty = cir::IntType::get(&getMLIRContext(), 32, /*isSigned=*/false);
+ UInt64Ty = cir::IntType::get(&getMLIRContext(), 64, /*isSigned=*/false);
+ UInt128Ty = cir::IntType::get(&getMLIRContext(), 128, /*isSigned=*/false);
+}
mlir::Location CIRGenModule::getLoc(SourceLocation cLoc) {
assert(cLoc.isValid() && "expected valid source location");
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 7a84c942af4913..397e501fd4e873 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#include "CIRGenBuilder.h"
#include "CIRGenTypeCache.h"
#include "CIRGenTypes.h"
@@ -47,9 +48,7 @@ class CIRGenModule : public CIRGenTypeCache {
~CIRGenModule() = default;
private:
- // TODO(CIR) 'builder' will change to CIRGenBuilderTy once that type is
- // defined
- mlir::OpBuilder builder;
+ CIRGenBuilderTy builder;
/// Hold Clang AST information.
clang::ASTContext &astCtx;
@@ -67,9 +66,10 @@ class CIRGenModule : public CIRGenTypeCache {
public:
mlir::ModuleOp getModule() const { return theModule; }
- mlir::OpBuilder &getBuilder() { return builder; }
+ CIRGenBuilderTy &getBuilder() { return builder; }
clang::ASTContext &getASTContext() const { return astCtx; }
CIRGenTypes &getTypes() { return genTypes; }
+ mlir::MLIRContext &getMLIRContext() { return *builder.getContext(); }
/// Helpers to convert the presumed location of Clang's SourceLocation to an
/// MLIR Location.
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypeCache.h b/clang/lib/CIR/CodeGen/CIRGenTypeCache.h
index fde9a355f52416..a357663c33e0f8 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypeCache.h
+++ b/clang/lib/CIR/CodeGen/CIRGenTypeCache.h
@@ -13,6 +13,8 @@
#ifndef LLVM_CLANG_LIB_CIR_CIRGENTYPECACHE_H
#define LLVM_CLANG_LIB_CIR_CIRGENTYPECACHE_H
+#include "clang/CIR/Dialect/IR/CIRTypes.h"
+
namespace clang::CIRGen {
/// This structure provides a set of types that are commonly used
@@ -20,6 +22,20 @@ namespace clang::CIRGen {
/// constructor and then copied around into new CIRGenFunction's.
struct CIRGenTypeCache {
CIRGenTypeCache() = default;
+
+ // ClangIR signed integral types of common sizes
+ cir::IntType SInt8Ty;
+ cir::IntType SInt16Ty;
+ cir::IntType SInt32Ty;
+ cir::IntType SInt64Ty;
+ cir::IntType SInt128Ty;
+
+ // ClangIR unsigned integral type of common sizes
+ cir::IntType UInt8Ty;
+ cir::IntType UInt16Ty;
+ cir::IntType UInt32Ty;
+ cir::IntType UInt64Ty;
+ cir::IntType UInt128Ty;
};
} // namespace clang::CIRGen
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
index e3fcbacf5f8108..e93bf93b1cb7d3 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.cpp
@@ -9,14 +9,24 @@ using namespace clang;
using namespace clang::CIRGen;
CIRGenTypes::CIRGenTypes(CIRGenModule &genModule)
- : cgm(genModule), context(genModule.getASTContext()) {}
+ : cgm(genModule), context(genModule.getASTContext()),
+ builder(cgm.getBuilder()) {}
CIRGenTypes::~CIRGenTypes() {}
+mlir::MLIRContext &CIRGenTypes::getMLIRContext() const {
+ return *builder.getContext();
+}
+
mlir::Type CIRGenTypes::convertType(QualType type) {
type = context.getCanonicalType(type);
const Type *ty = type.getTypePtr();
+ // Has the type already been processed?
+ TypeCacheTy::iterator tci = typeCache.find(ty);
+ if (tci != typeCache.end())
+ return tci->second;
+
// For types that haven't been implemented yet or are otherwise unsupported,
// report an error and return 'int'.
@@ -24,7 +34,7 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
switch (ty->getTypeClass()) {
case Type::Builtin: {
switch (cast<BuiltinType>(ty)->getKind()) {
- // Signed types.
+ // Signed integral types.
case BuiltinType::Char_S:
case BuiltinType::Int:
case BuiltinType::Int128:
@@ -33,11 +43,10 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
case BuiltinType::SChar:
case BuiltinType::Short:
case BuiltinType::WChar_S:
- resultType = cir::IntType::get(cgm.getBuilder().getContext(),
- context.getTypeSize(ty),
+ resultType = cir::IntType::get(&getMLIRContext(), context.getTypeSize(ty),
/*isSigned=*/true);
break;
- // Unsigned types.
+ // Unsigned integral types.
case BuiltinType::Char8:
case BuiltinType::Char16:
case BuiltinType::Char32:
@@ -49,14 +58,12 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
case BuiltinType::ULongLong:
case BuiltinType::UShort:
case BuiltinType::WChar_U:
- resultType = cir::IntType::get(cgm.getBuilder().getContext(),
- context.getTypeSize(ty),
+ resultType = cir::IntType::get(&getMLIRContext(), context.getTypeSize(ty),
/*isSigned=*/false);
break;
default:
cgm.errorNYI(SourceLocation(), "processing of built-in type", type);
- resultType = cir::IntType::get(cgm.getBuilder().getContext(), 32,
- /*isSigned=*/true);
+ resultType = cgm.SInt32Ty;
break;
}
break;
@@ -65,23 +72,21 @@ mlir::Type CIRGenTypes::convertType(QualType type) {
const auto *bitIntTy = cast<BitIntType>(type);
if (bitIntTy->getNumBits() > cir::IntType::maxBitwidth()) {
cgm.errorNYI(SourceLocation(), "large _BitInt type", type);
- resultType = cir::IntType::get(cgm.getBuilder().getContext(), 32,
- /*isSigned=*/true);
+ resultType = cgm.SInt32Ty;
} else {
- resultType =
- cir::IntType::get(cgm.getBuilder().getContext(),
- bitIntTy->getNumBits(), bitIntTy->isSigned());
+ resultType = cir::IntType::get(&getMLIRContext(), bitIntTy->getNumBits(),
+ bitIntTy->isSigned());
}
break;
}
default:
cgm.errorNYI(SourceLocation(), "processing of type", type);
- resultType =
- cir::IntType::get(cgm.getBuilder().getContext(), 32, /*isSigned=*/true);
+ resultType = cgm.SInt32Ty;
break;
}
assert(resultType && "Type conversion not yet implemented");
+ typeCache[ty] = resultType;
return resultType;
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenTypes.h b/clang/lib/CIR/CodeGen/CIRGenTypes.h
index b37738c770de1e..b5039b6d4a81db 100644
--- a/clang/lib/CIR/CodeGen/CIRGenTypes.h
+++ b/clang/lib/CIR/CodeGen/CIRGenTypes.h
@@ -15,9 +15,12 @@
#include "clang/CIR/Dialect/IR/CIRTypes.h"
+#include "llvm/ADT/SmallPtrSet.h"
+
namespace clang {
class ASTContext;
class QualType;
+class Type;
} // namespace clang
namespace mlir {
@@ -26,6 +29,7 @@ class Type;
namespace clang::CIRGen {
+class CIRGenBuilderTy;
class CIRGenModule;
/// This class organizes the cross-module state that is used while lowering
@@ -33,11 +37,19 @@ class CIRGenModule;
class CIRGenTypes {
CIRGenModule &cgm;
clang::ASTContext &context;
+ CIRGenBuilderTy &builder;
public:
CIRGenTypes(CIRGenModule &cgm);
~CIRGenTypes();
+ /// This map of clang::Type to mlir::Type (which includes CIR type) is a
+ /// cache of types that have already been processed.
+ using TypeCacheTy = llvm::DenseMap<const clang::Type *, mlir::Type>;
+ TypeCacheTy typeCache;
+
+ mlir::MLIRContext &getMLIRContext() const;
+
/// Convert a Clang type into a mlir::Type.
mlir::Type convertType(clang::QualType type);
};
|
erichkeane
reviewed
Dec 6, 2024
Collaborator
erichkeane
left a comment
erichkeane
left a comment
There was a problem hiding this comment.
1 question, else LGTM. @lanza or @bcardosolopes should be the one to approve this, as this is basically all directly CIR related.
| case Type::Builtin: { | ||
| switch (cast<BuiltinType>(ty)->getKind()) { | ||
| // Signed types. | ||
| // Signed integral types. |
Collaborator
There was a problem hiding this comment.
Perhaps some additional complexity, but was it considered to use the cached kinds for all of these instead of creating it again?
Contributor
Author
There was a problem hiding this comment.
I thought about it briefly, but decided that the code complexity was not worth the benefit. The typeCache being added in this commit means each cir::IntType will be duplicated only once or twice, not lots of times. So the benefits of using cgm.SInt* are minimal.
Contributor
Author
|
@lanza or @bcardosolopes : Could one of you please review and (hopefully) approve this PR? Erich has delegated approval to you for this one. |
lanza
approved these changes
Dec 10, 2024
Member
lanza
left a comment
lanza
left a comment
There was a problem hiding this comment.
LGTM with the two nits. @bcardosolopes anything to add?
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/195/builds/2197 Here is the relevant piece of the build log for the reference |
bcardosolopes
pushed a commit
to bcardosolopes/llvm-project
that referenced
this pull request
Mar 6, 2025
…lvm#119037)" This reverts commit ffb19f4.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Small infrastructure and background changes to ClangIR.
Create class
CIRGenBuilderTyand its base classCIRBaseBuilderTy. These are mostly empty for now, except for what is inherited frommlir::OpBuilder. But they will fill up quickly as more ClangIR code gen is upstreamed.CIRGenModuleandCIRGenTypesare changed to useCIRGenBuilderTy.Add cached types to struct
CIRGenTypeCachefor the integral types that are currently supported. Initialize those cached types in theCIRGenModuleconstructor. The first uses of those types (well, one of them) is inCIRGenTypes::convertType.Have
CIRGenTypes::convertTypecache its results in a map fromclang::Typetomlir::Type, saving it from having to convert the same type again and again.There are no new tests or changed tests in this commit. There are no changes to behavior, just improvements to how the existing behavior is implemented.