llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangir Author: Henrich Lauko (xlauko) <details> <summary>Changes</summary> - 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 --- Full diff: https://github.com/llvm/llvm-project/pull/141830.diff 6 Files Affected: - (modified) clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h (+27-18) - (modified) clang/lib/CIR/CodeGen/CIRGenBuilder.h (+4-11) - (modified) clang/lib/CIR/CodeGen/CIRGenModule.h (+1-1) - (modified) clang/lib/CIR/Dialect/IR/CIRAttrs.cpp (+1-2) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+16-24) - (modified) clang/unittests/CIR/PointerLikeTest.cpp (+3-5) ``````````diff 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 add1b40ab0aea..767b2f8e2a550 100644 --- a/clang/lib/CIR/CodeGen/CIRGenModule.h +++ b/clang/lib/CIR/CodeGen/CIRGenModule.h @@ -208,7 +208,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) { `````````` </details> https://github.com/llvm/llvm-project/pull/141830 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
