[clang] [CIR] Streamline creation of mlir::IntegerAttrs using mlir::Builder (PR #141830)
Henrich Lauko via cfe-commits
cfe-commits at lists.llvm.org
Thu May 29 03:26:14 PDT 2025
https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/141830
>From d4026ae9fabe8ee8a5d76a69ba5f89a7f1a7c3ff Mon Sep 17 00:00:00 2001
From: xlauko <xlauko at mail.muni.cz>
Date: Wed, 28 May 2025 21:15:58 +0200
Subject: [PATCH 1/3] [CIR] Streamline creation of mlir::IntegerAttrs using
mlir::Builder
- Uses getI<bitwidth>IntegerAttr builder method instead of explicit attribute and its type creation.
- Adds few helper functions `getAlignmentAttr` to build alignment representing mlir::IntegerAttr.
- Removes duplicit type parameters, that are inferred from mlir::IntegerAttr.
This mirrors incubator changes from https://github.com/llvm/clangir/pull/1645#event-17840237927
---
.../CIR/Dialect/Builder/CIRBaseBuilder.h | 45 +++++++++++--------
clang/lib/CIR/CodeGen/CIRGenBuilder.h | 15 ++-----
clang/lib/CIR/CodeGen/CIRGenModule.h | 2 +-
clang/lib/CIR/Dialect/IR/CIRAttrs.cpp | 3 +-
.../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 40 +++++++----------
clang/unittests/CIR/PointerLikeTest.cpp | 8 ++--
6 files changed, 52 insertions(+), 61 deletions(-)
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index 9de3a66755778..ac65c66c01589 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -18,6 +18,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "mlir/IR/Builders.h"
+#include "mlir/IR/BuiltinAttributes.h"
#include "mlir/IR/BuiltinTypes.h"
#include "mlir/IR/Location.h"
#include "mlir/IR/Types.h"
@@ -167,9 +168,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
}
mlir::TypedAttr getConstPtrAttr(mlir::Type type, int64_t value) {
- auto valueAttr = mlir::IntegerAttr::get(
- mlir::IntegerType::get(type.getContext(), 64), value);
- return cir::ConstPtrAttr::get(type, valueAttr);
+ return cir::ConstPtrAttr::get(type, getI64IntegerAttr(value));
}
mlir::Value createAlloca(mlir::Location loc, cir::PointerType addrType,
@@ -197,14 +196,9 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
mlir::Value createDummyValue(mlir::Location loc, mlir::Type type,
clang::CharUnits alignment) {
- auto addr = createAlloca(loc, getPointerTo(type), type, {},
- getSizeFromCharUnits(getContext(), alignment));
- mlir::IntegerAttr alignAttr;
- uint64_t align = alignment.getQuantity();
- if (align)
- alignAttr = getI64IntegerAttr(align);
-
- return create<cir::LoadOp>(loc, addr, /*isDeref=*/false, alignAttr);
+ mlir::IntegerAttr alignmentAttr = getAlignmentAttr(alignment);
+ auto addr = createAlloca(loc, getPointerTo(type), type, {}, alignmentAttr);
+ return create<cir::LoadOp>(loc, addr, /*isDeref=*/false, alignmentAttr);
}
cir::PtrStrideOp createPtrStride(mlir::Location loc, mlir::Value base,
@@ -428,13 +422,28 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
return OpBuilder::InsertPoint(block, block->begin());
};
- mlir::IntegerAttr getSizeFromCharUnits(mlir::MLIRContext *ctx,
- clang::CharUnits size) {
- // Note that mlir::IntegerType is used instead of cir::IntType here
- // because we don't need sign information for this to be useful, so keep
- // it simple.
- return mlir::IntegerAttr::get(mlir::IntegerType::get(ctx, 64),
- size.getQuantity());
+ //
+ // Alignement and size helpers
+ //
+
+ // Note that mlir::IntegerType is used instead of cir::IntType here because we
+ // don't need sign information for these to be useful, so keep it simple.
+
+ // Fot 0 alignment, return an empty attribute.
+ mlir::IntegerAttr getAlignmentAttr(clang::CharUnits alignment) {
+ return getAlignmentAttr(alignment.getQuantity());
+ }
+
+ mlir::IntegerAttr getAlignmentAttr(llvm::Align alignment) {
+ return getAlignmentAttr(alignment.value());
+ }
+
+ mlir::IntegerAttr getAlignmentAttr(int64_t alignment) {
+ return alignment ? getI64IntegerAttr(alignment) : mlir::IntegerAttr();
+ }
+
+ mlir::IntegerAttr getSizeFromCharUnits(clang::CharUnits size) {
+ return getI64IntegerAttr(size.getQuantity());
}
/// Create a loop condition.
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index e53f5e9202961..b8548487d5b31 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -282,22 +282,15 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
cir::LoadOp createLoad(mlir::Location loc, Address addr,
bool isVolatile = false) {
- mlir::IntegerAttr align;
- uint64_t alignment = addr.getAlignment().getQuantity();
- if (alignment)
- align = getI64IntegerAttr(alignment);
+ mlir::IntegerAttr align = getAlignmentAttr(addr.getAlignment());
return create<cir::LoadOp>(loc, addr.getPointer(), /*isDeref=*/false,
align);
}
cir::StoreOp createStore(mlir::Location loc, mlir::Value val, Address dst,
- ::mlir::IntegerAttr align = {}) {
- if (!align) {
- uint64_t alignment = dst.getAlignment().getQuantity();
- if (alignment)
- align = mlir::IntegerAttr::get(mlir::IntegerType::get(getContext(), 64),
- alignment);
- }
+ mlir::IntegerAttr align = {}) {
+ if (!align)
+ align = getAlignmentAttr(dst.getAlignment());
return CIRBaseBuilderTy::createStore(loc, val, dst.getPointer(), align);
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index aac8fe72b1890..5c716e2ab9e04 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -212,7 +212,7 @@ class CIRGenModule : public CIRGenTypeCache {
const clang::FunctionDecl *funcDecl);
mlir::IntegerAttr getSize(CharUnits size) {
- return builder.getSizeFromCharUnits(&getMLIRContext(), size);
+ return builder.getSizeFromCharUnits(size);
}
const llvm::Triple &getTriple() const { return target.getTriple(); }
diff --git a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
index c4fb4942aec75..52f0b33afaba4 100644
--- a/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRAttrs.cpp
@@ -64,8 +64,7 @@ void CIRDialect::printAttribute(Attribute attr, DialectAsmPrinter &os) const {
static ParseResult parseConstPtr(AsmParser &parser, mlir::IntegerAttr &value) {
if (parser.parseOptionalKeyword("null").succeeded()) {
- value = mlir::IntegerAttr::get(
- mlir::IntegerType::get(parser.getContext(), 64), 0);
+ value = parser.getBuilder().getI64IntegerAttr(0);
return success();
}
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index 8e82af7e62bc0..d30c85d572fed 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -417,8 +417,7 @@ mlir::LogicalResult CIRToLLVMCastOpLowering::matchAndRewrite(
case cir::CastKind::int_to_bool: {
mlir::Value llvmSrcVal = adaptor.getOperands().front();
mlir::Value zeroInt = rewriter.create<mlir::LLVM::ConstantOp>(
- castOp.getLoc(), llvmSrcVal.getType(),
- mlir::IntegerAttr::get(llvmSrcVal.getType(), 0));
+ castOp.getLoc(), llvmSrcVal.getType(), 0);
rewriter.replaceOpWithNewOp<mlir::LLVM::ICmpOp>(
castOp, mlir::LLVM::ICmpPredicate::ne, llvmSrcVal, zeroInt);
break;
@@ -630,9 +629,8 @@ mlir::LogicalResult CIRToLLVMPtrStrideOpLowering::matchAndRewrite(
if (rewriteSub) {
index = rewriter.create<mlir::LLVM::SubOp>(
index.getLoc(), index.getType(),
- rewriter.create<mlir::LLVM::ConstantOp>(
- index.getLoc(), index.getType(),
- mlir::IntegerAttr::get(index.getType(), 0)),
+ rewriter.create<mlir::LLVM::ConstantOp>(index.getLoc(),
+ index.getType(), 0),
index);
rewriter.eraseOp(sub);
}
@@ -648,8 +646,7 @@ mlir::LogicalResult CIRToLLVMAllocaOpLowering::matchAndRewrite(
mlir::ConversionPatternRewriter &rewriter) const {
assert(!cir::MissingFeatures::opAllocaDynAllocSize());
mlir::Value size = rewriter.create<mlir::LLVM::ConstantOp>(
- op.getLoc(), typeConverter->convertType(rewriter.getIndexType()),
- rewriter.getIntegerAttr(rewriter.getIndexType(), 1));
+ op.getLoc(), typeConverter->convertType(rewriter.getIndexType()), 1);
mlir::Type elementTy =
convertTypeForMemory(*getTypeConverter(), dataLayout, op.getAllocaType());
mlir::Type resultTy = convertTypeForMemory(*getTypeConverter(), dataLayout,
@@ -1111,18 +1108,16 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite(
switch (op.getKind()) {
case cir::UnaryOpKind::Inc: {
assert(!isVector && "++ not allowed on vector types");
- mlir::LLVM::ConstantOp one = rewriter.create<mlir::LLVM::ConstantOp>(
- loc, llvmType, mlir::IntegerAttr::get(llvmType, 1));
+ auto one = rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, 1);
rewriter.replaceOpWithNewOp<mlir::LLVM::AddOp>(
op, llvmType, adaptor.getInput(), one, maybeNSW);
return mlir::success();
}
case cir::UnaryOpKind::Dec: {
assert(!isVector && "-- not allowed on vector types");
- mlir::LLVM::ConstantOp one = rewriter.create<mlir::LLVM::ConstantOp>(
- loc, llvmType, mlir::IntegerAttr::get(llvmType, 1));
- rewriter.replaceOpWithNewOp<mlir::LLVM::SubOp>(
- op, llvmType, adaptor.getInput(), one, maybeNSW);
+ auto one = rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, 1);
+ rewriter.replaceOpWithNewOp<mlir::LLVM::SubOp>(op, adaptor.getInput(),
+ one, maybeNSW);
return mlir::success();
}
case cir::UnaryOpKind::Plus:
@@ -1133,10 +1128,9 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite(
if (isVector)
zero = rewriter.create<mlir::LLVM::ZeroOp>(loc, llvmType);
else
- zero = rewriter.create<mlir::LLVM::ConstantOp>(
- loc, llvmType, mlir::IntegerAttr::get(llvmType, 0));
+ zero = rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, 0);
rewriter.replaceOpWithNewOp<mlir::LLVM::SubOp>(
- op, llvmType, zero, adaptor.getInput(), maybeNSW);
+ op, zero, adaptor.getInput(), maybeNSW);
return mlir::success();
}
case cir::UnaryOpKind::Not: {
@@ -1150,11 +1144,10 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite(
minusOne =
rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, denseVec);
} else {
- minusOne = rewriter.create<mlir::LLVM::ConstantOp>(
- loc, llvmType, mlir::IntegerAttr::get(llvmType, -1));
+ minusOne = rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, -1);
}
- rewriter.replaceOpWithNewOp<mlir::LLVM::XOrOp>(
- op, llvmType, adaptor.getInput(), minusOne);
+ rewriter.replaceOpWithNewOp<mlir::LLVM::XOrOp>(op, adaptor.getInput(),
+ minusOne);
return mlir::success();
}
}
@@ -1206,10 +1199,9 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite(
return op.emitError() << "Unsupported unary operation on boolean type";
case cir::UnaryOpKind::Not: {
assert(!isVector && "NYI: op! on vector mask");
- mlir::LLVM::ConstantOp one = rewriter.create<mlir::LLVM::ConstantOp>(
- loc, llvmType, rewriter.getIntegerAttr(llvmType, 1));
- rewriter.replaceOpWithNewOp<mlir::LLVM::XOrOp>(op, llvmType,
- adaptor.getInput(), one);
+ auto one = rewriter.create<mlir::LLVM::ConstantOp>(loc, llvmType, 1);
+ rewriter.replaceOpWithNewOp<mlir::LLVM::XOrOp>(op, adaptor.getInput(),
+ one);
return mlir::success();
}
}
diff --git a/clang/unittests/CIR/PointerLikeTest.cpp b/clang/unittests/CIR/PointerLikeTest.cpp
index c0da271d56d4c..22690f2834cc4 100644
--- a/clang/unittests/CIR/PointerLikeTest.cpp
+++ b/clang/unittests/CIR/PointerLikeTest.cpp
@@ -47,12 +47,10 @@ class CIROpenACCPointerLikeTest : public ::testing::Test {
llvm::StringMap<unsigned> recordNames;
mlir::IntegerAttr getAlignOne(mlir::MLIRContext *ctx) {
- // Note that mlir::IntegerType is used instead of cir::IntType here
- // because we don't need sign information for this to be useful, so keep
- // it simple.
+ // Note that mlir::IntegerType is used instead of cir::IntType here because
+ // we don't need sign information for this to be useful, so keep it simple.
clang::CharUnits align = clang::CharUnits::One();
- return mlir::IntegerAttr::get(mlir::IntegerType::get(ctx, 64),
- align.getQuantity());
+ return b.getI64IntegerAttr(align.getQuantity());
}
mlir::StringAttr getUniqueRecordName(const std::string &baseName) {
>From ac27f4b77a64325604b56b897eaa98ef824f1ce4 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <xlauko at mail.muni.cz>
Date: Thu, 29 May 2025 12:16:30 +0200
Subject: [PATCH 2/3] Update
clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
Co-authored-by: Andy Kaylor <akaylor at nvidia.com>
---
clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index ac65c66c01589..ae100ee2eb563 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -429,7 +429,7 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
// Note that mlir::IntegerType is used instead of cir::IntType here because we
// don't need sign information for these to be useful, so keep it simple.
- // Fot 0 alignment, return an empty attribute.
+ // For 0 alignment, return an empty attribute.
mlir::IntegerAttr getAlignmentAttr(clang::CharUnits alignment) {
return getAlignmentAttr(alignment.getQuantity());
}
>From 41b6128a060de0a1b7d9aca0aa69734b7cc263e4 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <xlauko at mail.muni.cz>
Date: Thu, 29 May 2025 12:19:08 +0200
Subject: [PATCH 3/3] Update
clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
Co-authored-by: Erich Keane <ekeane at nvidia.com>
---
clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index ae100ee2eb563..e9d8a2baedf2f 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -423,13 +423,14 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
};
//
- // Alignement and size helpers
+ // Alignment and size helpers
//
// Note that mlir::IntegerType is used instead of cir::IntType here because we
// don't need sign information for these to be useful, so keep it simple.
- // For 0 alignment, return an empty attribute.
+ // For 0 alignment, any overload of `getAlignmentAttr` returns an empty
+ // attribute.
mlir::IntegerAttr getAlignmentAttr(clang::CharUnits alignment) {
return getAlignmentAttr(alignment.getQuantity());
}
More information about the cfe-commits
mailing list