Skip to content

[CIR] Generate the nsw flag correctly for unary ops #133815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 2, 2025
Merged

[CIR] Generate the nsw flag correctly for unary ops #133815

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a96e075
[CIR] Generate the nsw flag correctly for unary ops
andykaylor Mar 31, 2025
c1650db
Add round trip test and documentation
andykaylor Apr 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions clang/include/clang/CIR/Dialect/IR/CIROps.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -702,10 +702,14 @@ def UnaryOp : CIR_Op<"unary", [Pure, SameOperandsAndResultType]> {
}];

let results = (outs CIR_AnyType:$result);
let arguments = (ins Arg<UnaryOpKind, "unary op kind">:$kind, Arg<CIR_AnyType>:$input);
let arguments = (ins Arg<UnaryOpKind, "unary op kind">:$kind,
Copy link
Collaborator

@erichkeane erichkeane Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be documented in the description since we're adding a new argument?

Copy link
Contributor Author

@andykaylor andykaylor Apr 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. Good catch.

Arg<CIR_AnyType>:$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;
Expand Down
1 change: 0 additions & 1 deletion clang/include/clang/CIR/MissingFeatures.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 9 additions & 10 deletions clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
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<PointerType>(type)) {
cgf.cgm.errorNYI(e->getSourceRange(), "Unary inc/dec pointer");
Expand Down Expand Up @@ -429,19 +429,17 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
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 {};
}
Expand Down Expand Up @@ -473,18 +471,19 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
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<cir::UnaryOp>(
cgf.getLoc(e->getSourceRange().getBegin()), input.getType(), kind,
input);
input, nsw);
}

mlir::Value VisitUnaryNot(const UnaryOperator *e) {
Expand Down
10 changes: 2 additions & 8 deletions clang/lib/CIR/Lowering/DirectToLLVM/LowerToLLVM.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -860,14 +860,8 @@ mlir::LogicalResult CIRToLLVMUnaryOpLowering::matchAndRewrite(
// Integer unary operations: + - ~ ++ --
if (mlir::isa<cir::IntType>(elementType)) {
mlir::LLVM::IntegerOverflowFlags maybeNSW =
mlir::LLVM::IntegerOverflowFlags::none;
if (mlir::dyn_cast<cir::IntType>(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");
Expand Down
33 changes: 26 additions & 7 deletions clang/test/CIR/CodeGen/unary.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]]

Expand Down Expand Up @@ -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()
Expand All @@ -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]]

Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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
}