llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Henrich Lauko (xlauko) <details> <summary>Changes</summary> Replace the monolithic cir.binop.overflow operation and its BinOpOverflowKind enum with three individual operations: cir.add.overflow, cir.sub.overflow, and cir.mul.overflow. This follows the same pattern used when BinOp and UnaryOp were previously split into per-operation ops (cir.add, cir.sub, etc.), eliminating enum dispatch and enabling per-op traits like Commutative. --- Patch is 44.85 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/186653.diff 6 Files Affected: - (modified) clang/include/clang/CIR/Dialect/IR/CIROps.td (+62-47) - (modified) clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp (+44-36) - (modified) clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp (+6-6) - (modified) clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp (+36-48) - (modified) clang/test/CIR/CodeGen/new.cpp (+7-7) - (modified) clang/test/CIR/CodeGenBuiltins/builtins-overflow.cpp (+30-30) ``````````diff diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index e7cb11be08f01..2dfe8635b1ded 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -2154,44 +2154,30 @@ def CIR_CmpOp : CIR_Op<"cmp", [Pure, SameTypeOperands]> { } //===----------------------------------------------------------------------===// -// BinOpOverflowOp +// Checked Binary Arithmetic Operations (with overflow) //===----------------------------------------------------------------------===// -def CIR_BinOpOverflowKind : CIR_I32EnumAttr< - "BinOpOverflowKind", "checked binary arithmetic operation kind", [ - I32EnumAttrCase<"Add", 0, "add">, - I32EnumAttrCase<"Sub", 1, "sub">, - I32EnumAttrCase<"Mul", 2, "mul"> -]>; - -def CIR_BinOpOverflowOp : CIR_Op<"binop.overflow", [Pure, SameTypeOperands]> { - let summary = "Perform binary integral arithmetic with overflow checking"; - let description = [{ - `cir.binop.overflow` performs binary arithmetic operations with overflow - checking on integral operands. - - The `kind` argument specifies the kind of arithmetic operation to perform. - It can be either `add`, `sub`, or `mul`. The `lhs` and `rhs` arguments - specify the input operands of the arithmetic operation. The types of `lhs` - and `rhs` must be the same. - - `cir.binop.overflow` produces two SSA values. `result` is the result of the - arithmetic operation truncated to its specified type. `overflow` is a - boolean value indicating whether overflow happens during the operation. - - The exact semantic of this operation is as follows: - - - `lhs` and `rhs` are promoted to an imaginary integral type that has - infinite precision. - - The arithmetic operation is performed on the promoted operands. - - The infinite-precision result is truncated to the type of `result`. The - truncated result is assigned to `result`. - - If the truncated result is equal to the un-truncated result, `overflow` - is assigned to false. Otherwise, `overflow` is assigned to true. - }]; +// Base class for binary arithmetic operations with overflow checking. +// +// Each op produces two SSA values: `result` is the arithmetic result truncated +// to its specified type, and `overflow` is a boolean indicating whether +// overflow occurred. +// +// The exact semantics are: +// - `lhs` and `rhs` are promoted to an imaginary infinite-precision +// integral type. +// - The arithmetic operation is performed on the promoted operands. +// - The infinite-precision result is truncated to the type of `result`. +// - `overflow` is true if the truncated result differs from the +// infinite-precision result. +// +// The type of `result` may differ from the operand type. This models C +// builtins like `__builtin_*_overflow` where operands and the result can +// have different widths or signedness. +class CIR_BinOpOverflow<string mnemonic, list<Trait> traits = []> + : CIR_Op<mnemonic, !listconcat([Pure, SameTypeOperands], traits)> { let arguments = (ins - CIR_BinOpOverflowKind:$kind, CIR_IntType:$lhs, CIR_IntType:$rhs ); @@ -2199,32 +2185,61 @@ def CIR_BinOpOverflowOp : CIR_Op<"binop.overflow", [Pure, SameTypeOperands]> { let results = (outs CIR_IntType:$result, CIR_BoolType:$overflow); let assemblyFormat = [{ - `(` $kind `,` $lhs `,` $rhs `)` `:` qualified(type($lhs)) `,` - `(` qualified(type($result)) `,` qualified(type($overflow)) `)` + $lhs `,` $rhs `:` qualified(type($lhs)) `->` qualified(type($result)) attr-dict }]; let builders = [ OpBuilder<(ins "cir::IntType":$resultTy, - "cir::BinOpOverflowKind":$kind, "mlir::Value":$lhs, "mlir::Value":$rhs), [{ auto overflowTy = cir::BoolType::get($_builder.getContext()); - build($_builder, $_state, resultTy, overflowTy, kind, lhs, rhs); + build($_builder, $_state, resultTy, overflowTy, lhs, rhs); }]> ]; - let extraLLVMLoweringPatternDecl = [{ - static std::string getLLVMIntrinName(cir::BinOpOverflowKind opKind, - bool isSigned, unsigned width); +} - struct EncompassedTypeInfo { - bool sign; - unsigned width; - }; +def CIR_AddOverflowOp : CIR_BinOpOverflow<"add.overflow", [Commutative]> { + let summary = "Integer addition with overflow checking"; + let description = [{ + `cir.add.overflow` performs addition with overflow checking on integral + operands. See `CIR_BinOpOverflow` for semantics. + + Example: - static EncompassedTypeInfo computeEncompassedTypeWidth(cir::IntType operandTy, - cir::IntType resultTy); + ```mlir + %result, %overflow = cir.add.overflow %a, %b : !u32i -> !u32i + %result, %overflow = cir.add.overflow %a, %b : !cir.int<s, 33> -> !s32i + ``` + }]; +} + +def CIR_SubOverflowOp : CIR_BinOpOverflow<"sub.overflow"> { + let summary = "Integer subtraction with overflow checking"; + let description = [{ + `cir.sub.overflow` performs subtraction with overflow checking on integral + operands. See `CIR_BinOpOverflow` for semantics. + + Example: + + ```mlir + %result, %overflow = cir.sub.overflow %a, %b : !u32i -> !u32i + ``` + }]; +} + +def CIR_MulOverflowOp : CIR_BinOpOverflow<"mul.overflow", [Commutative]> { + let summary = "Integer multiplication with overflow checking"; + let description = [{ + `cir.mul.overflow` performs multiplication with overflow checking on + integral operands. See `CIR_BinOpOverflow` for semantics. + + Example: + + ```mlir + %result, %overflow = cir.mul.overflow %a, %b : !u32i -> !u32i + ``` }]; } diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp index f155365ab5de2..3fd18d5204bad 100644 --- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp @@ -231,6 +231,16 @@ getIntegerWidthAndSignedness(const clang::ASTContext &astContext, return {width, isSigned}; } +/// Create a checked overflow arithmetic op and return its result and overflow +/// flag. +template <typename OpTy> +static std::pair<mlir::Value, mlir::Value> +emitOverflowOp(CIRGenBuilderTy &builder, mlir::Location loc, + cir::IntType resultTy, mlir::Value lhs, mlir::Value rhs) { + auto op = OpTy::create(builder, loc, resultTy, lhs, rhs); + return {op.getResult(), op.getOverflow()}; +} + // Given one or more integer types, this function produces an integer type that // encompasses them: any value in one of the given types could be expressed in // the encompassing type. @@ -1900,38 +1910,36 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, &getMLIRContext(), encompassingInfo.width, encompassingInfo.isSigned); auto resultCIRTy = mlir::cast<cir::IntType>(cgm.convertType(resultQTy)); - mlir::Value left = emitScalarExpr(leftArg); - mlir::Value right = emitScalarExpr(rightArg); + mlir::Value x = emitScalarExpr(leftArg); + mlir::Value y = emitScalarExpr(rightArg); Address resultPtr = emitPointerWithAlignment(resultArg); // Extend each operand to the encompassing type, if necessary. - if (left.getType() != encompassingCIRTy) - left = - builder.createCast(cir::CastKind::integral, left, encompassingCIRTy); - if (right.getType() != encompassingCIRTy) - right = - builder.createCast(cir::CastKind::integral, right, encompassingCIRTy); + if (x.getType() != encompassingCIRTy) + x = builder.createCast(cir::CastKind::integral, x, encompassingCIRTy); + if (y.getType() != encompassingCIRTy) + y = builder.createCast(cir::CastKind::integral, y, encompassingCIRTy); // Perform the operation on the extended values. - cir::BinOpOverflowKind opKind; + mlir::Location loc = getLoc(e->getSourceRange()); + mlir::Value result, overflow; switch (builtinID) { default: llvm_unreachable("Unknown overflow builtin id."); case Builtin::BI__builtin_add_overflow: - opKind = cir::BinOpOverflowKind::Add; + std::tie(result, overflow) = + emitOverflowOp<cir::AddOverflowOp>(builder, loc, resultCIRTy, x, y); break; case Builtin::BI__builtin_sub_overflow: - opKind = cir::BinOpOverflowKind::Sub; + std::tie(result, overflow) = + emitOverflowOp<cir::SubOverflowOp>(builder, loc, resultCIRTy, x, y); break; case Builtin::BI__builtin_mul_overflow: - opKind = cir::BinOpOverflowKind::Mul; + std::tie(result, overflow) = + emitOverflowOp<cir::MulOverflowOp>(builder, loc, resultCIRTy, x, y); break; } - mlir::Location loc = getLoc(e->getSourceRange()); - auto arithOp = cir::BinOpOverflowOp::create(builder, loc, resultCIRTy, - opKind, left, right); - // Here is a slight difference from the original clang CodeGen: // - In the original clang CodeGen, the checked arithmetic result is // first computed as a value of the encompassing type, and then it is @@ -1944,9 +1952,9 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, // Finally, store the result using the pointer. bool isVolatile = resultArg->getType()->getPointeeType().isVolatileQualified(); - builder.createStore(loc, arithOp.getResult(), resultPtr, isVolatile); + builder.createStore(loc, result, resultPtr, isVolatile); - return RValue::get(arithOp.getOverflow()); + return RValue::get(overflow); } case Builtin::BI__builtin_uadd_overflow: @@ -1968,14 +1976,19 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, case Builtin::BI__builtin_smull_overflow: case Builtin::BI__builtin_smulll_overflow: { // Scalarize our inputs. - mlir::Value x = emitScalarExpr(e->getArg(0)); - mlir::Value y = emitScalarExpr(e->getArg(1)); + mlir::Value lhs = emitScalarExpr(e->getArg(0)); + mlir::Value rhs = emitScalarExpr(e->getArg(1)); const clang::Expr *resultArg = e->getArg(2); Address resultPtr = emitPointerWithAlignment(resultArg); - // Decide which of the arithmetic operation we are lowering to: - cir::BinOpOverflowKind arithKind; + clang::QualType resultQTy = + resultArg->getType()->castAs<clang::PointerType>()->getPointeeType(); + auto resultCIRTy = mlir::cast<cir::IntType>(cgm.convertType(resultQTy)); + + // Create the appropriate overflow-checked arithmetic operation. + mlir::Location loc = getLoc(e->getSourceRange()); + mlir::Value result, overflow; switch (builtinID) { default: llvm_unreachable("Unknown overflow builtin id."); @@ -1985,7 +1998,8 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, case Builtin::BI__builtin_sadd_overflow: case Builtin::BI__builtin_saddl_overflow: case Builtin::BI__builtin_saddll_overflow: - arithKind = cir::BinOpOverflowKind::Add; + std::tie(result, overflow) = + emitOverflowOp<cir::AddOverflowOp>(builder, loc, resultCIRTy, x, y); break; case Builtin::BI__builtin_usub_overflow: case Builtin::BI__builtin_usubl_overflow: @@ -1993,7 +2007,8 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, case Builtin::BI__builtin_ssub_overflow: case Builtin::BI__builtin_ssubl_overflow: case Builtin::BI__builtin_ssubll_overflow: - arithKind = cir::BinOpOverflowKind::Sub; + std::tie(result, overflow) = + emitOverflowOp<cir::SubOverflowOp>(builder, loc, resultCIRTy, x, y); break; case Builtin::BI__builtin_umul_overflow: case Builtin::BI__builtin_umull_overflow: @@ -2001,24 +2016,17 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, case Builtin::BI__builtin_smul_overflow: case Builtin::BI__builtin_smull_overflow: case Builtin::BI__builtin_smulll_overflow: - arithKind = cir::BinOpOverflowKind::Mul; + std::tie(result, overflow) = + emitOverflowOp<cir::MulOverflowOp>(builder, loc, resultCIRTy, x, y); break; } - clang::QualType resultQTy = - resultArg->getType()->castAs<clang::PointerType>()->getPointeeType(); - auto resultCIRTy = mlir::cast<cir::IntType>(cgm.convertType(resultQTy)); - - mlir::Location loc = getLoc(e->getSourceRange()); - cir::BinOpOverflowOp arithOp = cir::BinOpOverflowOp::create( - builder, loc, resultCIRTy, arithKind, x, y); - bool isVolatile = resultArg->getType()->getPointeeType().isVolatileQualified(); - builder.createStore(loc, emitToMemory(arithOp.getResult(), resultQTy), - resultPtr, isVolatile); + builder.createStore(loc, emitToMemory(result, resultQTy), resultPtr, + isVolatile); - return RValue::get(arithOp.getOverflow()); + return RValue::get(overflow); } case Builtin::BIaddressof: diff --git a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp index 50617a8d04f6d..e565b029da9df 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprCXX.cpp @@ -581,9 +581,9 @@ static mlir::Value emitCXXNewAllocSize(CIRGenFunction &cgf, const CXXNewExpr *e, // allocation fails. if (typeSizeMultiplier != 1) { mlir::Value tsmV = cgf.getBuilder().getConstInt(loc, typeSizeMultiplier); - auto mulOp = cir::BinOpOverflowOp::create( - cgf.getBuilder(), loc, mlir::cast<cir::IntType>(cgf.sizeTy), - cir::BinOpOverflowKind::Mul, size, tsmV); + auto mulOp = cir::MulOverflowOp::create( + cgf.getBuilder(), loc, mlir::cast<cir::IntType>(cgf.sizeTy), size, + tsmV); if (hasOverflow) hasOverflow = @@ -617,9 +617,9 @@ static mlir::Value emitCXXNewAllocSize(CIRGenFunction &cgf, const CXXNewExpr *e, if (cookieSize != 0) { sizeWithoutCookie = size; mlir::Value cookieSizeV = cgf.getBuilder().getConstInt(loc, cookieSize); - auto addOp = cir::BinOpOverflowOp::create( - cgf.getBuilder(), loc, mlir::cast<cir::IntType>(cgf.sizeTy), - cir::BinOpOverflowKind::Add, size, cookieSizeV); + auto addOp = cir::AddOverflowOp::create( + cgf.getBuilder(), loc, mlir::cast<cir::IntType>(cgf.sizeTy), size, + cookieSizeV); if (hasOverflow) hasOverflow = diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index 4a1b4292b23dd..44d0dab36c7dd 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -3136,22 +3136,29 @@ mlir::LogicalResult CIRToLLVMCmpOpLowering::matchAndRewrite( return cmpOp.emitError() << "unsupported type for CmpOp: " << type; } -mlir::LogicalResult CIRToLLVMBinOpOverflowOpLowering::matchAndRewrite( - cir::BinOpOverflowOp op, OpAdaptor adaptor, - mlir::ConversionPatternRewriter &rewriter) const { +/// Shared lowering logic for checked binary arithmetic overflow operations. +/// The \p opStr parameter specifies the arithmetic operation name used in the +/// LLVM intrinsic (e.g., "add", "sub", "mul"). +template <typename OpTy> +static mlir::LogicalResult +lowerBinOpOverflow(OpTy op, typename OpTy::Adaptor adaptor, + mlir::ConversionPatternRewriter &rewriter, + const mlir::TypeConverter *typeConverter, + llvm::StringRef opStr) { mlir::Location loc = op.getLoc(); - cir::BinOpOverflowKind arithKind = op.getKind(); cir::IntType operandTy = op.getLhs().getType(); cir::IntType resultTy = op.getResult().getType(); - EncompassedTypeInfo encompassedTyInfo = - computeEncompassedTypeWidth(operandTy, resultTy); - mlir::IntegerType encompassedLLVMTy = - rewriter.getIntegerType(encompassedTyInfo.width); + bool sign = operandTy.getIsSigned() || resultTy.getIsSigned(); + unsigned width = + std::max(operandTy.getWidth() + (sign && operandTy.isUnsigned()), + resultTy.getWidth() + (sign && resultTy.isUnsigned())); + + mlir::IntegerType encompassedLLVMTy = rewriter.getIntegerType(width); mlir::Value lhs = adaptor.getLhs(); mlir::Value rhs = adaptor.getRhs(); - if (operandTy.getWidth() < encompassedTyInfo.width) { + if (operandTy.getWidth() < width) { if (operandTy.isSigned()) { lhs = mlir::LLVM::SExtOp::create(rewriter, loc, encompassedLLVMTy, lhs); rhs = mlir::LLVM::SExtOp::create(rewriter, loc, encompassedLLVMTy, rhs); @@ -3161,8 +3168,10 @@ mlir::LogicalResult CIRToLLVMBinOpOverflowOpLowering::matchAndRewrite( } } - std::string intrinName = getLLVMIntrinName(arithKind, encompassedTyInfo.sign, - encompassedTyInfo.width); + // The intrinsic name is `@llvm.{s|u}{op}.with.overflow.i{width}` + std::string intrinName = ("llvm." + llvm::Twine(sign ? 's' : 'u') + opStr + + ".with.overflow.i" + llvm::Twine(width)) + .str(); auto intrinNameAttr = mlir::StringAttr::get(op.getContext(), intrinName); mlir::IntegerType overflowLLVMTy = rewriter.getI1Type(); @@ -3180,8 +3189,8 @@ mlir::LogicalResult CIRToLLVMBinOpOverflowOpLowering::matchAndRewrite( rewriter, loc, intrinRet, ArrayRef<int64_t>{1}) .getResult(); - if (resultTy.getWidth() < encompassedTyInfo.width) { - mlir::Type resultLLVMTy = getTypeConverter()->convertType(resultTy); + if (resultTy.getWidth() < width) { + mlir::Type resultLLVMTy = typeConverter->convertType(resultTy); auto truncResult = mlir::LLVM::TruncOp::create(rewriter, loc, resultLLVMTy, result); @@ -3202,7 +3211,7 @@ mlir::LogicalResult CIRToLLVMBinOpOverflowOpLowering::matchAndRewrite( } mlir::Type boolLLVMTy = - getTypeConverter()->convertType(op.getOverflow().getType()); + typeConverter->convertType(op.getOverflow().getType()); if (boolLLVMTy != rewriter.getI1Type()) overflow = mlir::LLVM::ZExtOp::create(rewriter, loc, boolLLVMTy, overflow); @@ -3211,43 +3220,22 @@ mlir::LogicalResult CIRToLLVMBinOpOverflowOpLowering::matchAndRewrite( return mlir::success(); } -std::string CIRToLLVMBinOpOverflowOpLowering::getLLVMIntrinName( - cir::BinOpOverflowKind opKind, bool isSigned, unsigned width) { - // The intrinsic name is `@llvm.{s|u}{opKind}.with.overflow.i{width}` - - std::string name = "llvm."; - - if (isSigned) - name.push_back('s'); - else - name.push_back('u'); - - switch (opKind) { - case cir::BinOpOverflowKind::Add: - name.append("add."); - break; - case cir::BinOpOverflowKind::Sub: - name.append("sub."); - break; - case cir::BinOpOverflowKind::Mul: - name.append("mul."); - break; - } - - name.append("with.overflow.i"); - name.append(std::to_string(width)); +mlir::LogicalResult CIRToLLVMAddOverflowOpLowering::matchAndRewrite( + cir::AddOverflowOp op, OpAdaptor adaptor, + mlir::ConversionPatternRewriter &rewriter) const { + return lowerBinOpOverflow(op, adaptor, rewriter, getTypeConverter(), "add"); +} - return name; +mlir::LogicalResult CIRToLLVMSubOverflowOpLowering::matchAndRewrite( + cir::SubOverflowOp op, OpAdaptor adaptor, + mlir::ConversionPatternRewriter &rewriter) const { + return lowerBinOpOverflow(op, adaptor, rewriter, getTypeConverter(), "sub"); } -CIRToLLVMBinOpOverflowOpLowering::EncompassedTypeInfo -CIRToLLVMBinOpOverflowOpLowering::computeEncompassedTypeWidth( - cir::IntType operandTy, cir::IntType resultTy) { - bool sign = operandTy.getIsSigned() || resultTy.getIsSigned(); - unsigned width = - std::max(operandTy.getWidth() + (sign && operandTy.isUnsigned()), - resultTy.getWidth() + (sign && resultTy.isUnsigned())); - return {sign, width}; +mlir::LogicalResult CIRToLLVMMulOverflowOpLowering::matchAndRewrite( + cir::MulOverflowOp op, OpAdaptor adaptor, + mlir::ConversionPatternRewriter &rewriter) const { + return lowerBinOpOverflow(op, adaptor, rewriter, getTypeConverter(), "mul"); } mlir::LogicalResult CIRToLLVMShiftOpLoweri... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/186653 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
