[CIR] Upstream overflow builtins #166643
Open
[CIR] Upstream overflow builtins #166643
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
This implements the builtins that handle overflow.
adams381
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
Member
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: None (adams381) ChangesThis implements the builtins that handle overflow. Patch is 37.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/166643.diff 5 Files Affected:
diff --git a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
index 3288f5b12c77e..6c1951714ba1f 100644
--- a/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
+++ b/clang/include/clang/CIR/Dialect/Builder/CIRBaseBuilder.h
@@ -408,6 +408,20 @@ class CIRBaseBuilderTy : public mlir::OpBuilder {
callee.getFunctionType().getReturnType(), operands);
}
+ struct BinOpOverflowResults {
+ mlir::Value result;
+ mlir::Value overflow;
+ };
+
+ BinOpOverflowResults createBinOpOverflowOp(mlir::Location loc,
+ cir::IntType resultTy,
+ cir::BinOpOverflowKind kind,
+ mlir::Value lhs, mlir::Value rhs) {
+ auto op =
+ cir::BinOpOverflowOp::create(*this, loc, resultTy, kind, lhs, rhs);
+ return {op.getResult(), op.getOverflow()};
+ }
+
//===--------------------------------------------------------------------===//
// Cast/Conversion Operators
//===--------------------------------------------------------------------===//
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index dc56db1bbd4ea..328880d6f3581 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1628,6 +1628,82 @@ def CIR_CmpOp : CIR_Op<"cmp", [Pure, SameTypeOperands]> {
let isLLVMLoweringRecursive = true;
}
+//===----------------------------------------------------------------------===//
+// BinOpOverflowOp
+//===----------------------------------------------------------------------===//
+
+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.
+ }];
+
+ let arguments = (ins
+ CIR_BinOpOverflowKind:$kind,
+ CIR_IntType:$lhs,
+ CIR_IntType:$rhs
+ );
+
+ let results = (outs CIR_IntType:$result, CIR_BoolType:$overflow);
+
+ let assemblyFormat = [{
+ `(` $kind `,` $lhs `,` $rhs `)` `:` type($lhs) `,`
+ `(` type($result) `,` type($overflow) `)`
+ 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);
+ }]>
+ ];
+
+ let extraLLVMLoweringPatternDecl = [{
+ static std::string getLLVMIntrinName(cir::BinOpOverflowKind opKind,
+ bool isSigned, unsigned width);
+
+ struct EncompassedTypeInfo {
+ bool sign;
+ unsigned width;
+ };
+
+ static EncompassedTypeInfo computeEncompassedTypeWidth(cir::IntType operandTy,
+ cir::IntType resultTy);
+ }];
+}
+
+
//===----------------------------------------------------------------------===//
// BinOp
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
index d9b9e3b877b50..19ce15ca5aeeb 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenBuiltin.cpp
@@ -58,6 +58,52 @@ static RValue emitBuiltinBitOp(CIRGenFunction &cgf, const CallExpr *e,
return RValue::get(result);
}
+namespace {
+struct WidthAndSignedness {
+ unsigned Width;
+ bool Signed;
+};
+} // namespace
+
+static WidthAndSignedness
+getIntegerWidthAndSignedness(const clang::ASTContext &astContext,
+ const clang::QualType Type) {
+ assert(Type->isIntegerType() && "Given type is not an integer.");
+ unsigned Width = Type->isBooleanType() ? 1
+ : Type->isBitIntType() ? astContext.getIntWidth(Type)
+ : astContext.getTypeInfo(Type).Width;
+ bool Signed = Type->isSignedIntegerType();
+ return {Width, Signed};
+}
+
+// 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.
+static struct WidthAndSignedness
+EncompassingIntegerType(ArrayRef<struct WidthAndSignedness> Types) {
+ assert(Types.size() > 0 && "Empty list of types.");
+
+ // If any of the given types is signed, we must return a signed type.
+ bool Signed = false;
+ for (const auto &Type : Types) {
+ Signed |= Type.Signed;
+ }
+
+ // The encompassing type must have a width greater than or equal to the width
+ // of the specified types. Additionally, if the encompassing type is signed,
+ // its width must be strictly greater than the width of any unsigned types
+ // given.
+ unsigned Width = 0;
+ for (const auto &Type : Types) {
+ unsigned MinWidth = Type.Width + (Signed && !Type.Signed);
+ if (Width < MinWidth) {
+ Width = MinWidth;
+ }
+ }
+
+ return {Width, Signed};
+}
+
RValue CIRGenFunction::emitRotate(const CallExpr *e, bool isRotateLeft) {
mlir::Value input = emitScalarExpr(e->getArg(0));
mlir::Value amount = emitScalarExpr(e->getArg(1));
@@ -491,6 +537,154 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID,
cir::PrefetchOp::create(builder, loc, address, locality, isWrite);
return RValue::get(nullptr);
}
+ case Builtin::BI__builtin_add_overflow:
+ case Builtin::BI__builtin_sub_overflow:
+ case Builtin::BI__builtin_mul_overflow: {
+ const clang::Expr *LeftArg = e->getArg(0);
+ const clang::Expr *RightArg = e->getArg(1);
+ const clang::Expr *ResultArg = e->getArg(2);
+
+ clang::QualType ResultQTy =
+ ResultArg->getType()->castAs<clang::PointerType>()->getPointeeType();
+
+ WidthAndSignedness LeftInfo =
+ getIntegerWidthAndSignedness(cgm.getASTContext(), LeftArg->getType());
+ WidthAndSignedness RightInfo =
+ getIntegerWidthAndSignedness(cgm.getASTContext(), RightArg->getType());
+ WidthAndSignedness ResultInfo =
+ getIntegerWidthAndSignedness(cgm.getASTContext(), ResultQTy);
+
+ // Note we compute the encompassing type with the consideration to the
+ // result type, so later in LLVM lowering we don't get redundant integral
+ // extension casts.
+ WidthAndSignedness EncompassingInfo =
+ EncompassingIntegerType({LeftInfo, RightInfo, ResultInfo});
+
+ auto EncompassingCIRTy = cir::IntType::get(
+ &getMLIRContext(), EncompassingInfo.Width, EncompassingInfo.Signed);
+ auto ResultCIRTy = mlir::cast<cir::IntType>(cgm.convertType(ResultQTy));
+
+ mlir::Value Left = emitScalarExpr(LeftArg);
+ mlir::Value Right = 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);
+
+ // Perform the operation on the extended values.
+ cir::BinOpOverflowKind OpKind;
+ switch (builtinID) {
+ default:
+ llvm_unreachable("Unknown overflow builtin id.");
+ case Builtin::BI__builtin_add_overflow:
+ OpKind = cir::BinOpOverflowKind::Add;
+ break;
+ case Builtin::BI__builtin_sub_overflow:
+ OpKind = cir::BinOpOverflowKind::Sub;
+ break;
+ case Builtin::BI__builtin_mul_overflow:
+ OpKind = cir::BinOpOverflowKind::Mul;
+ break;
+ }
+
+ auto Loc = getLoc(e->getSourceRange());
+ auto ArithResult =
+ builder.createBinOpOverflowOp(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
+ // truncated to the actual result type with a second overflow checking.
+ // - In CIRGen, the checked arithmetic operation directly produce the
+ // checked arithmetic result in its expected type.
+ //
+ // So we don't need a truncation and a second overflow checking here.
+
+ // Finally, store the result using the pointer.
+ bool isVolatile =
+ ResultArg->getType()->getPointeeType().isVolatileQualified();
+ builder.createStore(Loc, emitToMemory(ArithResult.result, ResultQTy),
+ ResultPtr, isVolatile);
+
+ return RValue::get(ArithResult.overflow);
+ }
+
+ case Builtin::BI__builtin_uadd_overflow:
+ case Builtin::BI__builtin_uaddl_overflow:
+ case Builtin::BI__builtin_uaddll_overflow:
+ case Builtin::BI__builtin_usub_overflow:
+ case Builtin::BI__builtin_usubl_overflow:
+ case Builtin::BI__builtin_usubll_overflow:
+ case Builtin::BI__builtin_umul_overflow:
+ case Builtin::BI__builtin_umull_overflow:
+ case Builtin::BI__builtin_umulll_overflow:
+ case Builtin::BI__builtin_sadd_overflow:
+ case Builtin::BI__builtin_saddl_overflow:
+ case Builtin::BI__builtin_saddll_overflow:
+ case Builtin::BI__builtin_ssub_overflow:
+ case Builtin::BI__builtin_ssubl_overflow:
+ case Builtin::BI__builtin_ssubll_overflow:
+ case Builtin::BI__builtin_smul_overflow:
+ 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));
+
+ const clang::Expr *ResultArg = e->getArg(2);
+ Address ResultPtr = emitPointerWithAlignment(ResultArg);
+
+ // Decide which of the arithmetic operation we are lowering to:
+ cir::BinOpOverflowKind ArithKind;
+ switch (builtinID) {
+ default:
+ llvm_unreachable("Unknown overflow builtin id.");
+ case Builtin::BI__builtin_uadd_overflow:
+ case Builtin::BI__builtin_uaddl_overflow:
+ case Builtin::BI__builtin_uaddll_overflow:
+ case Builtin::BI__builtin_sadd_overflow:
+ case Builtin::BI__builtin_saddl_overflow:
+ case Builtin::BI__builtin_saddll_overflow:
+ ArithKind = cir::BinOpOverflowKind::Add;
+ break;
+ case Builtin::BI__builtin_usub_overflow:
+ case Builtin::BI__builtin_usubl_overflow:
+ case Builtin::BI__builtin_usubll_overflow:
+ case Builtin::BI__builtin_ssub_overflow:
+ case Builtin::BI__builtin_ssubl_overflow:
+ case Builtin::BI__builtin_ssubll_overflow:
+ ArithKind = cir::BinOpOverflowKind::Sub;
+ break;
+ case Builtin::BI__builtin_umul_overflow:
+ case Builtin::BI__builtin_umull_overflow:
+ case Builtin::BI__builtin_umulll_overflow:
+ case Builtin::BI__builtin_smul_overflow:
+ case Builtin::BI__builtin_smull_overflow:
+ case Builtin::BI__builtin_smulll_overflow:
+ ArithKind = cir::BinOpOverflowKind::Mul;
+ break;
+ }
+
+ clang::QualType ResultQTy =
+ ResultArg->getType()->castAs<clang::PointerType>()->getPointeeType();
+ auto ResultCIRTy = mlir::cast<cir::IntType>(cgm.convertType(ResultQTy));
+
+ auto Loc = getLoc(e->getSourceRange());
+ auto ArithResult =
+ builder.createBinOpOverflowOp(Loc, ResultCIRTy, ArithKind, X, Y);
+
+ bool isVolatile =
+ ResultArg->getType()->getPointeeType().isVolatileQualified();
+ builder.createStore(Loc, emitToMemory(ArithResult.result, ResultQTy),
+ ResultPtr, isVolatile);
+
+ return RValue::get(ArithResult.overflow);
+ }
}
// If this is an alias for a lib function (e.g. __builtin_sin), emit
diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
index d94108294a9a3..c81f7cc657137 100644
--- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
+++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
@@ -2503,6 +2503,118 @@ 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 {
+ auto loc = op.getLoc();
+ auto arithKind = op.getKind();
+ auto operandTy = op.getLhs().getType();
+ auto resultTy = op.getResult().getType();
+
+ auto encompassedTyInfo = computeEncompassedTypeWidth(operandTy, resultTy);
+ auto encompassedLLVMTy = rewriter.getIntegerType(encompassedTyInfo.width);
+
+ auto lhs = adaptor.getLhs();
+ auto rhs = adaptor.getRhs();
+ if (operandTy.getWidth() < encompassedTyInfo.width) {
+ if (operandTy.isSigned()) {
+ lhs = rewriter.create<mlir::LLVM::SExtOp>(loc, encompassedLLVMTy, lhs);
+ rhs = rewriter.create<mlir::LLVM::SExtOp>(loc, encompassedLLVMTy, rhs);
+ } else {
+ lhs = rewriter.create<mlir::LLVM::ZExtOp>(loc, encompassedLLVMTy, lhs);
+ rhs = rewriter.create<mlir::LLVM::ZExtOp>(loc, encompassedLLVMTy, rhs);
+ }
+ }
+
+ auto intrinName = getLLVMIntrinName(arithKind, encompassedTyInfo.sign,
+ encompassedTyInfo.width);
+ auto intrinNameAttr = mlir::StringAttr::get(op.getContext(), intrinName);
+
+ auto overflowLLVMTy = rewriter.getI1Type();
+ auto intrinRetTy = mlir::LLVM::LLVMStructType::getLiteral(
+ rewriter.getContext(), {encompassedLLVMTy, overflowLLVMTy});
+
+ auto callLLVMIntrinOp = rewriter.create<mlir::LLVM::CallIntrinsicOp>(
+ loc, intrinRetTy, intrinNameAttr, mlir::ValueRange{lhs, rhs});
+ auto intrinRet = callLLVMIntrinOp.getResult(0);
+
+ auto result = rewriter
+ .create<mlir::LLVM::ExtractValueOp>(loc, intrinRet,
+ ArrayRef<int64_t>{0})
+ .getResult();
+ auto overflow = rewriter
+ .create<mlir::LLVM::ExtractValueOp>(loc, intrinRet,
+ ArrayRef<int64_t>{1})
+ .getResult();
+
+ if (resultTy.getWidth() < encompassedTyInfo.width) {
+ auto resultLLVMTy = getTypeConverter()->convertType(resultTy);
+ auto truncResult =
+ rewriter.create<mlir::LLVM::TruncOp>(loc, resultLLVMTy, result);
+
+ // Extend the truncated result back to the encompassing type to check for
+ // any overflows during the truncation.
+ mlir::Value truncResultExt;
+ if (resultTy.isSigned())
+ truncResultExt = rewriter.create<mlir::LLVM::SExtOp>(
+ loc, encompassedLLVMTy, truncResult);
+ else
+ truncResultExt = rewriter.create<mlir::LLVM::ZExtOp>(
+ loc, encompassedLLVMTy, truncResult);
+ auto truncOverflow = rewriter.create<mlir::LLVM::ICmpOp>(
+ loc, mlir::LLVM::ICmpPredicate::ne, truncResultExt, result);
+
+ result = truncResult;
+ overflow = rewriter.create<mlir::LLVM::OrOp>(loc, overflow, truncOverflow);
+ }
+
+ auto boolLLVMTy = getTypeConverter()->convertType(op.getOverflow().getType());
+ if (boolLLVMTy != rewriter.getI1Type())
+ overflow = rewriter.create<mlir::LLVM::ZExtOp>(loc, boolLLVMTy, overflow);
+
+ rewriter.replaceOp(op, mlir::ValueRange{result, overflow});
+
+ 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));
+
+ return name;
+}
+
+CIRToLLVMBinOpOverflowOpLowering::EncompassedTypeInfo
+CIRToLLVMBinOpOverflowOpLowering::computeEncompassedTypeWidth(
+ cir::IntType operandTy, cir::IntType resultTy) {
+ auto sign = operandTy.getIsSigned() || resultTy.getIsSigned();
+ auto width = std::max(operandTy.getWidth() + (sign && operandTy.isUnsigned()),
+ resultTy.getWidth() + (sign && resultTy.isUnsigned()));
+ return {sign, width};
+}
+
mlir::LogicalResult CIRToLLVMShiftOpLowering::matchAndRewrite(
cir::ShiftOp op, OpAdaptor adaptor,
mlir::ConversionPatternRewriter &rewriter) const {
diff --git a/clang/test/CIR/CodeGen/builtins-overflow.cpp b/clang/test/CIR/CodeGen/builtins-overflow.cpp
new file mode 100644
index 0000000000000..8cd227d58686d
--- /dev/null
+++ b/clang/test/CIR/CodeGen/builtins-overflow.cpp
@@ -0,0 +1,364 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir
+// RUN: FileCheck %s --check-prefix=CIR --input-file=%t.cir
+
+bool test_add_overflow_uint_uint_uint(unsigned x, unsigned y, unsigned *res) {
+ return __builtin_add_overflow(x, y, res);
+}
+
+// CIR: cir.func dso_local @_Z32test_add_overflow_uint_uint_uintjjPj
+// CIR: %[[#LHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!u32i>, !u32i
+// CIR-NEXT: %[[#RHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!u32i>, !u32i
+// CIR-NEXT: %[[#RES_PTR:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!cir.ptr<!u32i>>, !cir.ptr<!u32i>
+// CIR-NEXT: %[[RES:.+]], %{{.+}} = cir.binop.overflow(add, %[[#LHS]], %[[#RHS]]) : !u32i, (!u32i, !cir.bool)
+// CIR-NEXT: cir.store{{.*}} %[[RES]], %[[#RES_PTR]] : !u32i, !cir.ptr<!u32i>
+// CIR: }
+
+bool test_add_overflow_int_int_int(int x, int y, int *res) {
+ return __builtin_add_overflow(x, y, res);
+}
+
+// CIR: cir.func dso_local @_Z29test_add_overflow_int_int_intiiPi
+// CIR: %[[#LHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!s32i>, !s32i
+// CIR-NEXT: %[[#RHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!s32i>, !s32i
+// CIR-NEXT: %[[#RES_PTR:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!cir.ptr<!s32i>>, !cir.ptr<!s32i>
+// CIR-NEXT: %[[RES:.+]], %{{.+}} = cir.binop.overflow(add, %[[#LHS]], %[[#RHS]]) : !s32i, (!s32i, !cir.bool)
+// CIR-NEXT: cir.store{{.*}} %[[RES]], %[[#RES_PTR]] : !s32i, !cir.ptr<!s32i>
+// CIR: }
+
+bool test_add_overflow_xint31_xint31_xint31(_BitInt(31) x, _BitInt(31) y, _BitInt(31) *res) {
+ return __builtin_add_overflow(x, y, res);
+}
+
+// CIR: cir.func dso_local @_Z38test_add_overflow_xint31_xint31_xint31DB31_S_PS_
+// CIR: %[[#LHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!cir.int<s, 31>>, !cir.int<s, 31>
+// CIR-NEXT: %[[#RHS:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!cir.int<s, 31>>, !cir.int<s, 31>
+// CIR-NEXT: %[[#RES_PTR:]] = cir.load{{.*}} %{{.+}} : !cir.ptr<!cir.ptr<!cir.int<s, 31>>>, !cir.ptr<!cir.int<s, 31>>
+// CIR-NEXT: %[[RES:.+]], %{{.+}} = cir.binop....
[truncated]
|
xlauko
reviewed
Nov 5, 2025
Contributor
xlauko
left a comment
xlauko
left a comment
There was a problem hiding this comment.
Hi @adams381 thank you for your first contribution. I will have few general notes on your PR. As we are upstreaming incubator code we are alining it more with upstream clang code standards. That is for instance:
- use of auto. General rule of thumb is to use it only in case the type is explicitly alredy used on the line, e.g.
auto x = cast<SomeType>(y) - lower camelCase for variable names
Can you please update the PR before moving forward.
xlauko
reviewed
Nov 5, 2025
andykaylor
reviewed
Nov 6, 2025
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Remove the BinOpOverflowResults struct and createBinOpOverflowOp helper function from CIRBaseBuilder. Instead, call cir::BinOpOverflowOp::create directly and use getResult() and getOverflow() on the returned operation. This simplifies the API and makes it more natural to use, as suggested by reviewer feedback.
Replace auto with explicit types and use lowerCamelCase. Replace deprecated rewriter.create with Op::create.
- Rename 'signed' field to 'isSigned' (signed is a keyword) - Use lowerCamelCase for all variables and parameters - Replace createBinOpOverflowOp helper with direct BinOpOverflowOp::create calls
Author
|
Thanks for your feedback. I have addressed all review comments: |
andykaylor
reviewed
Nov 19, 2025
Contributor
andykaylor
left a comment
andykaylor
left a comment
There was a problem hiding this comment.
It looks like some of your changes may not have made it into the update.
| break; | ||
| } | ||
|
|
||
| auto Loc = getLoc(e->getSourceRange()); |
Contributor
There was a problem hiding this comment.
This still needs to be addressed.
xlauko
reviewed
Nov 19, 2025
xlauko
reviewed
Nov 19, 2025
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Andy Kaylor <akaylor@nvidia.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Co-authored-by: Henrich Lauko <henrich.lau@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
This implements the builtins that handle overflow.
This fixes issue #163888