-
Notifications
You must be signed in to change notification settings - Fork 15.4k
feat: included support for is_constant builtin #166832
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.
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
Changes from 3 commits
227f9ad
f2d9b64
62223b1
267c6ba
32665a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1702,6 +1702,47 @@ def CIR_BinOp : CIR_Op<"binop", [ | |||||
| }]; | ||||||
| } | ||||||
|
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
| // BinOpOverflow | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| def CIR_BinOpOverflow : CIR_Op<"binop.overflow", [Pure]> { | ||||||
| let summary = "Binary operations with overflow detection"; | ||||||
| let description = [{ | ||||||
| cir.binop.overflow performs the binary operation according to | ||||||
| the specified opcode kind (add, sub, or mul) and returns both | ||||||
| the result and an overflow flag. | ||||||
|
|
||||||
| It requires two input operands and returns two results: | ||||||
| - The result of the operation (same type as operands) | ||||||
| - An overflow flag (boolean type) | ||||||
|
|
||||||
| The operation supports both signed and unsigned integer types. | ||||||
| For signed types, overflow is detected when the result cannot | ||||||
| be represented in the result type. For unsigned types, overflow | ||||||
| is detected when the result wraps around. | ||||||
|
|
||||||
| ```mlir | ||||||
| %result, %overflow = cir.binop.overflow(add, %1, %2) : !s32i -> (!s32i, !bool) | ||||||
| %result, %overflow = cir.binop.overflow(sub, %3, %4) : !u32i -> (!u32i, !bool) | ||||||
| %result, %overflow = cir.binop.overflow(mul, %5, %6) : !s64i -> (!s64i, !bool) | ||||||
| ``` | ||||||
| }]; | ||||||
|
|
||||||
| let arguments = (ins | ||||||
| CIR_BinOpKind:$kind, | ||||||
| CIR_AnyType:$lhs, CIR_AnyType:$rhs | ||||||
| ); | ||||||
|
|
||||||
| let results = (outs CIR_AnyType:$result, CIR_BoolType:$overflow); | ||||||
|
|
||||||
| let assemblyFormat = [{ | ||||||
| `(` $kind `,` $lhs `,` $rhs `)` `:` type($lhs) `->` `(` type($result) `,` type($overflow) `)` attr-dict | ||||||
| }]; | ||||||
|
|
||||||
| let hasVerifier = 1; | ||||||
| } | ||||||
|
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
| // ShiftOp | ||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
@@ -4052,6 +4093,44 @@ def CIR_ExpectOp : CIR_Op<"expect", [ | |||||
| }]; | ||||||
| } | ||||||
|
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
| // IsConstantOp | ||||||
|
||||||
| //===----------------------------------------------------------------------===// | ||||||
|
|
||||||
| def CIR_IsConstantOp : CIR_Op<"is_constant", [Pure]> { | ||||||
| let summary = "Check if a value is a compile-time constant"; | ||||||
| let description = [{ | ||||||
|
||||||
| The `cir.is_constant` operation checks whether its input value is a | ||||||
| compile-time constant. This operation models the `__builtin_constant_p` | ||||||
| builtin function. | ||||||
|
|
||||||
| The operation takes a single operand of any CIR type and returns a signed | ||||||
| 32-bit integer. The result is 1 if the operand is a compile-time constant, | ||||||
| and 0 otherwise. | ||||||
|
|
||||||
| If the value can be determined to be constant at compile time, this | ||||||
| operation may be folded to a constant value. Otherwise, it will be lowered | ||||||
| to the `llvm.is.constant` intrinsic. | ||||||
|
|
||||||
| Example: | ||||||
|
|
||||||
| ```mlir | ||||||
| %0 = cir.is_constant %expr : i32 -> !s32i | ||||||
| %1 = cir.is_constant %ptr : !cir.ptr<i32> -> !s32i | ||||||
| ``` | ||||||
| }]; | ||||||
|
|
||||||
| let arguments = (ins CIR_AnyType:$value); | ||||||
| let results = (outs CIR_SInt32:$result); | ||||||
|
||||||
| let results = (outs CIR_SInt32:$result); | |
| let results = (outs CIR_BoolType:$result); |
Why did you change this from what is in the incubator?
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xlauko Is this consistent with the style we're using now. I know you've been updating things recently, but I don't have a good sense for the expected style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andykaylor pretty much yes. I am traying us to converge on style : argument types -> return types respectivelly : type if the operation can be determined by a single type. Bit of style guide is in: https://llvm.github.io/clangir/Dialect/cir-style-guide.html
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be lowered to LLVM IR. The lowering is simple enough to include in this PR.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -199,6 +199,31 @@ RValue CIRGenFunction::emitBuiltinExpr(const GlobalDecl &gd, unsigned builtinID, | |||||
| return RValue::get( | ||||||
| builder.createBitcast(allocaAddr, builder.getVoidPtrTy())); | ||||||
| } | ||||||
|
|
||||||
| case Builtin::BI__builtin_constant_p: { | ||||||
| auto loc = getLoc(e->getSourceRange()); | ||||||
|
||||||
| auto loc = getLoc(e->getSourceRange()); | |
| mlir::Location loc = getLoc(e->getSourceRange()); |
Upstream LLVM uses auto less liberally than the incubator.
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've diverged from both the incubator and classic codegen here. That's OK if your changes improve the code while keeping the same semantics, but I don't think what you've done here meets those criteria. We need this to behave exactly like classic codegen. Please use the code from the incubator.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2117,6 +2117,25 @@ OpFoldResult cir::UnaryOp::fold(FoldAdaptor adaptor) { | |
| return {}; | ||
| } | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // IsConstantOp Definitions | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| OpFoldResult cir::IsConstantOp::fold(FoldAdaptor adaptor) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Folder is turned off in the operation, please set |
||
| // If the input value is a constant attribute, return 1 (true) | ||
| mlir::Attribute value = adaptor.getValue(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tested this? It doesn't look right. |
||
| if (value) { | ||
| // The value is a compile-time constant, so return 1 | ||
| mlir::Type resultType = getResult().getType(); | ||
| llvm::APInt apInt(32, 1); | ||
| llvm::APSInt apSInt(apInt, /*isUnsigned=*/false); | ||
| return cir::IntAttr::get(resultType, apSInt); | ||
| } | ||
|
|
||
| // If the input is not a constant, we cannot fold | ||
| return {}; | ||
| } | ||
|
|
||
| //===----------------------------------------------------------------------===// | ||
| // CopyOp Definitions | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't needed for this PR. Even if you were adding handlers for both sets of builtins, that would need to be done in separate PRs. We have a policy of keeping PRs focused on a single feature or bug fix.