From a96e0755319f8d18870187443a886b7c28e70fea Mon Sep 17 00:00:00 2001 From: Andy Kaylor Date: Mon, 31 Mar 2025 15:42:44 -0700 Subject: [PATCH 1/2] [CIR] Generate the nsw flag correctly for unary ops A previous checkin used a workaround to generate the nsw flag where needed for unary ops. This change upstreams a subsequent change that was made in the incubator to generate the flag correctly. --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 8 +++-- clang/include/clang/CIR/MissingFeatures.h | 1 - clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp | 19 +++++------ .../CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp | 10 ++---- clang/test/CIR/CodeGen/unary.cpp | 33 +++++++++++++++---- 5 files changed, 43 insertions(+), 28 deletions(-) diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 455cc2b8b0277..33447e668cedd 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -702,10 +702,14 @@ def UnaryOp : CIR_Op<"unary", [Pure, SameOperandsAndResultType]> { }]; let results = (outs CIR_AnyType:$result); - let arguments = (ins Arg:$kind, Arg:$input); + let arguments = (ins Arg:$kind, + Arg:$input, + UnitAttr:$no_signed_wrap); let assemblyFormat = [{ - `(` $kind `,` $input `)` `:` type($input) `,` type($result) attr-dict + `(` $kind `,` $input `)` + (`nsw` $no_signed_wrap^)? + `:` type($input) `,` type($result) attr-dict }]; let hasVerifier = 1; diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h index 3a102d90aba8f..23bf826d19a69 100644 --- a/clang/include/clang/CIR/MissingFeatures.h +++ b/clang/include/clang/CIR/MissingFeatures.h @@ -76,7 +76,6 @@ struct MissingFeatures { static bool opScopeCleanupRegion() { return false; } // Unary operator handling - static bool opUnarySignedOverflow() { return false; } static bool opUnaryPromotionType() { return false; } // Clang early optimizations or things defered to LLVM lowering. diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp index 2cf92dfbf3a5b..5ac1dc1052c2e 100644 --- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp +++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp @@ -374,7 +374,7 @@ class ScalarExprEmitter : public StmtVisitor { cir::UnaryOpKind kind = e->isIncrementOp() ? cir::UnaryOpKind::Inc : cir::UnaryOpKind::Dec; // NOTE(CIR): clang calls CreateAdd but folds this to a unary op - value = emitUnaryOp(e, kind, input); + value = emitUnaryOp(e, kind, input, /*nsw=*/false); } } else if (isa(type)) { cgf.cgm.errorNYI(e->getSourceRange(), "Unary inc/dec pointer"); @@ -429,19 +429,17 @@ class ScalarExprEmitter : public StmtVisitor { mlir::Value emitIncDecConsiderOverflowBehavior(const UnaryOperator *e, mlir::Value inVal, bool isInc) { - assert(!cir::MissingFeatures::opUnarySignedOverflow()); cir::UnaryOpKind kind = e->isIncrementOp() ? cir::UnaryOpKind::Inc : cir::UnaryOpKind::Dec; switch (cgf.getLangOpts().getSignedOverflowBehavior()) { case LangOptions::SOB_Defined: - return emitUnaryOp(e, kind, inVal); + return emitUnaryOp(e, kind, inVal, /*nsw=*/false); case LangOptions::SOB_Undefined: assert(!cir::MissingFeatures::sanitizers()); - return emitUnaryOp(e, kind, inVal); - break; + return emitUnaryOp(e, kind, inVal, /*nsw=*/true); case LangOptions::SOB_Trapping: if (!e->canOverflow()) - return emitUnaryOp(e, kind, inVal); + return emitUnaryOp(e, kind, inVal, /*nsw=*/true); cgf.cgm.errorNYI(e->getSourceRange(), "inc/def overflow SOB_Trapping"); return {}; } @@ -473,18 +471,19 @@ class ScalarExprEmitter : public StmtVisitor { assert(!cir::MissingFeatures::opUnaryPromotionType()); mlir::Value operand = Visit(e->getSubExpr()); - assert(!cir::MissingFeatures::opUnarySignedOverflow()); + bool nsw = + kind == cir::UnaryOpKind::Minus && e->getType()->isSignedIntegerType(); // NOTE: LLVM codegen will lower this directly to either a FNeg // or a Sub instruction. In CIR this will be handled later in LowerToLLVM. - return emitUnaryOp(e, kind, operand); + return emitUnaryOp(e, kind, operand, nsw); } mlir::Value emitUnaryOp(const UnaryOperator *e, cir::UnaryOpKind kind, - mlir::Value input) { + mlir::Value input, bool nsw = false) { return builder.create( cgf.getLoc(e->getSourceRange().getBegin()), input.getType(), kind, - input); + input, nsw); } mlir::Value VisitUnaryNot(const UnaryOperator *e) { diff --git a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp index b19be53947f99..48dc09d151dcf 100644 --- a/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp +++ b/clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp @@ -860,14 +860,8 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite( // Integer unary operations: + - ~ ++ -- if (mlir::isa(elementType)) { mlir::LLVM::IntegerOverflowFlags maybeNSW = - mlir::LLVM::IntegerOverflowFlags::none; - if (mlir::dyn_cast(elementType).isSigned()) { - assert(!cir::MissingFeatures::opUnarySignedOverflow()); - // TODO: For now, assume signed overflow is undefined. We'll need to add - // an attribute to the unary op to control this. - maybeNSW = mlir::LLVM::IntegerOverflowFlags::nsw; - } - + op.getNoSignedWrap() ? mlir::LLVM::IntegerOverflowFlags::nsw + : mlir::LLVM::IntegerOverflowFlags::none; switch (op.getKind()) { case cir::UnaryOpKind::Inc: { assert(!isVector && "++ not allowed on vector types"); diff --git a/clang/test/CIR/CodeGen/unary.cpp b/clang/test/CIR/CodeGen/unary.cpp index 3e041e14ce177..ca47c1068e08d 100644 --- a/clang/test/CIR/CodeGen/unary.cpp +++ b/clang/test/CIR/CodeGen/unary.cpp @@ -83,7 +83,7 @@ int inc0() { // CHECK: %[[ATMP:.*]] = cir.const #cir.int<1> : !s32i // CHECK: cir.store %[[ATMP]], %[[A]] : !s32i // CHECK: %[[INPUT:.*]] = cir.load %[[A]] -// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[INPUT]]) +// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[INPUT]]) nsw // CHECK: cir.store %[[INCREMENTED]], %[[A]] // CHECK: %[[A_TO_OUTPUT:.*]] = cir.load %[[A]] @@ -111,8 +111,8 @@ int dec0() { // CHECK: %[[ATMP:.*]] = cir.const #cir.int<1> : !s32i // CHECK: cir.store %[[ATMP]], %[[A]] : !s32i // CHECK: %[[INPUT:.*]] = cir.load %[[A]] -// CHECK: %[[INCREMENTED:.*]] = cir.unary(dec, %[[INPUT]]) -// CHECK: cir.store %[[INCREMENTED]], %[[A]] +// CHECK: %[[DECREMENTED:.*]] = cir.unary(dec, %[[INPUT]]) nsw +// CHECK: cir.store %[[DECREMENTED]], %[[A]] // CHECK: %[[A_TO_OUTPUT:.*]] = cir.load %[[A]] // LLVM: define i32 @dec0() @@ -139,7 +139,7 @@ int inc1() { // CHECK: %[[ATMP:.*]] = cir.const #cir.int<1> : !s32i // CHECK: cir.store %[[ATMP]], %[[A]] : !s32i // CHECK: %[[INPUT:.*]] = cir.load %[[A]] -// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[INPUT]]) +// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[INPUT]]) nsw // CHECK: cir.store %[[INCREMENTED]], %[[A]] // CHECK: %[[A_TO_OUTPUT:.*]] = cir.load %[[A]] @@ -167,8 +167,8 @@ int dec1() { // CHECK: %[[ATMP:.*]] = cir.const #cir.int<1> : !s32i // CHECK: cir.store %[[ATMP]], %[[A]] : !s32i // CHECK: %[[INPUT:.*]] = cir.load %[[A]] -// CHECK: %[[INCREMENTED:.*]] = cir.unary(dec, %[[INPUT]]) -// CHECK: cir.store %[[INCREMENTED]], %[[A]] +// CHECK: %[[DECREMENTED:.*]] = cir.unary(dec, %[[INPUT]]) nsw +// CHECK: cir.store %[[DECREMENTED]], %[[A]] // CHECK: %[[A_TO_OUTPUT:.*]] = cir.load %[[A]] // LLVM: define i32 @dec1() @@ -197,7 +197,7 @@ int inc2() { // CHECK: %[[ATMP:.*]] = cir.const #cir.int<1> : !s32i // CHECK: cir.store %[[ATMP]], %[[A]] : !s32i // CHECK: %[[ATOB:.*]] = cir.load %[[A]] -// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[ATOB]]) +// CHECK: %[[INCREMENTED:.*]] = cir.unary(inc, %[[ATOB]]) nsw // CHECK: cir.store %[[INCREMENTED]], %[[A]] // CHECK: cir.store %[[ATOB]], %[[B]] // CHECK: %[[B_TO_OUTPUT:.*]] = cir.load %[[B]] @@ -405,3 +405,22 @@ float fpPostInc2() { // OGCG: store float %[[A_INC]], ptr %[[A]], align 4 // OGCG: store float %[[A_LOAD]], ptr %[[B]], align 4 // OGCG: %[[B_TO_OUTPUT:.*]] = load float, ptr %[[B]], align 4 + +void chars(char c) { +// CHECK: cir.func @chars + + int c1 = +c; + // CHECK: %[[PROMO:.*]] = cir.cast(integral, %{{.+}} : !s8i), !s32i + // CHECK: cir.unary(plus, %[[PROMO]]) : !s32i, !s32i + int c2 = -c; + // CHECK: %[[PROMO:.*]] = cir.cast(integral, %{{.+}} : !s8i), !s32i + // CHECK: cir.unary(minus, %[[PROMO]]) nsw : !s32i, !s32i + + // Chars can go through some integer promotion codegen paths even when not promoted. + // These should not have nsw attributes because the intermediate promotion makes the + // overflow defined behavior. + ++c; // CHECK: cir.unary(inc, %{{.+}}) : !s8i, !s8i + --c; // CHECK: cir.unary(dec, %{{.+}}) : !s8i, !s8i + c++; // CHECK: cir.unary(inc, %{{.+}}) : !s8i, !s8i + c--; // CHECK: cir.unary(dec, %{{.+}}) : !s8i, !s8i +} From c1650db84c4f0050072441bb000cf6dc550dfc06 Mon Sep 17 00:00:00 2001 From: Andy Kaylor Date: Tue, 1 Apr 2025 14:21:12 -0700 Subject: [PATCH 2/2] Add round trip test and documentation --- clang/include/clang/CIR/Dialect/IR/CIROps.td | 21 ++++++-- clang/test/CIR/IR/unary.cir | 50 ++++++++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 clang/test/CIR/IR/unary.cir diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td index 33447e668cedd..e1b1bbd678d22 100644 --- a/clang/include/clang/CIR/Dialect/IR/CIROps.td +++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td @@ -695,9 +695,12 @@ def UnaryOp : CIR_Op<"unary", [Pure, SameOperandsAndResultType]> { It requires one input operand and has one result, both types should be the same. + If the `nsw` (no signed wrap) attribute is present, the result is poison if + signed overflow occurs. + ```mlir %7 = cir.unary(inc, %1) : i32 -> i32 - %8 = cir.unary(dec, %2) : i32 -> i32 + %8 = cir.unary(dec, %2) nsw : i32 -> i32 ``` }]; @@ -868,9 +871,21 @@ def BinOp : CIR_Op<"binop", [Pure, It requires two input operands and has one result, all types should be the same. + If the `nsw` (no signed wrap) or `nuw` (no unsigned wrap) attributes are + present, the result is poison if signed or unsigned overflow occurs + (respectively). + + If the `sat` (saturated) attribute is present, the result is clamped to + the maximum value representatable by the type if it would otherwise + exceed that value and is clamped to the minimum representable value if + it would otherwise be below that value. + ```mlir - %7 = cir.binop(add, %1, %2) : !s32i - %7 = cir.binop(mul, %1, %2) : !u8i + %5 = cir.binop(add, %1, %2) : !s32i + %6 = cir.binop(mul, %1, %2) : !u8i + %7 = cir.binop(add, %1, %2) nsw : !s32i + %8 = cir.binop(add, %3, %4) nuw : !u32i + %9 = cir.binop(add, %1, %2) sat : !s32i ``` }]; diff --git a/clang/test/CIR/IR/unary.cir b/clang/test/CIR/IR/unary.cir new file mode 100644 index 0000000000000..f01121adc106e --- /dev/null +++ b/clang/test/CIR/IR/unary.cir @@ -0,0 +1,50 @@ +// RUN: cir-opt %s | FileCheck %s + +!s32i = !cir.int +!s64i = !cir.int +!u32i = !cir.int +!u64i = !cir.int + +module { + cir.func @test_unary_unsigned() { + %0 = cir.alloca !u32i, !cir.ptr, ["a"] {alignment = 4 : i64} + %1 = cir.load %0 : !cir.ptr, !u32i + %2 = cir.unary(plus, %1) : !u32i, !u32i + %3 = cir.unary(minus, %1) : !u32i, !u32i + %4 = cir.unary(not, %1) : !u32i, !u32i + %5 = cir.unary(inc, %1) : !u32i, !u32i + %6 = cir.unary(dec, %1) : !u32i, !u32i + cir.return + } +// CHECK: cir.func @test_unary_unsigned() { +// CHECK: %0 = cir.alloca !u32i, !cir.ptr, ["a"] {alignment = 4 : i64} +// CHECK: %1 = cir.load %0 : !cir.ptr, !u32i +// CHECK: %2 = cir.unary(plus, %1) : !u32i, !u32i +// CHECK: %3 = cir.unary(minus, %1) : !u32i, !u32i +// CHECK: %4 = cir.unary(not, %1) : !u32i, !u32i +// CHECK: %5 = cir.unary(inc, %1) : !u32i, !u32i +// CHECK: %6 = cir.unary(dec, %1) : !u32i, !u32i +// CHECK: cir.return +// CHECK: } + + cir.func @test_unary_signed() { + %0 = cir.alloca !s32i, !cir.ptr, ["a"] {alignment = 4 : i64} + %1 = cir.load %0 : !cir.ptr, !s32i + %2 = cir.unary(plus, %1) : !s32i, !s32i + %3 = cir.unary(minus, %1) nsw : !s32i, !s32i + %4 = cir.unary(not, %1) : !s32i, !s32i + %5 = cir.unary(inc, %1) nsw : !s32i, !s32i + %6 = cir.unary(dec, %1) nsw : !s32i, !s32i + cir.return + } +// CHECK: cir.func @test_unary_signed() { +// CHECK: %0 = cir.alloca !s32i, !cir.ptr, ["a"] {alignment = 4 : i64} +// CHECK: %1 = cir.load %0 : !cir.ptr, !s32i +// CHECK: %2 = cir.unary(plus, %1) : !s32i, !s32i +// CHECK: %3 = cir.unary(minus, %1) nsw : !s32i, !s32i +// CHECK: %4 = cir.unary(not, %1) : !s32i, !s32i +// CHECK: %5 = cir.unary(inc, %1) nsw : !s32i, !s32i +// CHECK: %6 = cir.unary(dec, %1) nsw : !s32i, !s32i +// CHECK: cir.return +// CHECK: } +}