-
Notifications
You must be signed in to change notification settings - Fork 13.3k
[CIR] Fix unary op folding #132269
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
[CIR] Fix unary op folding #132269
Conversation
Unary ops had previously been omitted from the list of ops handled in the CIRCanonicalizePass. Although the incubator code doesn't use them directly, the mlir folding code does. This change enables folding of unary ops by adding them to the list.
@mmha This fixes the problem you noticed earlier. |
@llvm/pr-subscribers-clangir @llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesUnary ops had previously been omitted from the list of ops handled in the CIRCanonicalizePass. Although the incubator code doesn't use them directly, the mlir folding code does. This change enables folding of unary ops by adding them to the list. Full diff: https://github.com/llvm/llvm-project/pull/132269.diff 3 Files Affected:
diff --git a/clang/include/clang/CIR/MissingFeatures.h b/clang/include/clang/CIR/MissingFeatures.h
index d276af5686995..46780775dd4bd 100644
--- a/clang/include/clang/CIR/MissingFeatures.h
+++ b/clang/include/clang/CIR/MissingFeatures.h
@@ -110,7 +110,6 @@ struct MissingFeatures {
static bool brCondOp() { return false; }
static bool switchOp() { return false; }
static bool tryOp() { return false; }
- static bool unaryOp() { return false; }
static bool selectOp() { return false; }
static bool complexCreateOp() { return false; }
static bool complexRealOp() { return false; }
diff --git a/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp b/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
index d32691dba1473..6cbaf26a0bf33 100644
--- a/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/CIRCanonicalize.cpp
@@ -121,15 +121,14 @@ void CIRCanonicalizePass::runOnOperation() {
assert(!cir::MissingFeatures::brCondOp());
assert(!cir::MissingFeatures::switchOp());
assert(!cir::MissingFeatures::tryOp());
- assert(!cir::MissingFeatures::unaryOp());
assert(!cir::MissingFeatures::selectOp());
assert(!cir::MissingFeatures::complexCreateOp());
assert(!cir::MissingFeatures::complexRealOp());
assert(!cir::MissingFeatures::complexImagOp());
assert(!cir::MissingFeatures::callOp());
- // CastOp here is to perform a manual `fold` in
- // applyOpPatternsGreedily
- if (isa<BrOp, ScopeOp, CastOp>(op))
+ // CastOp and UnaryOp are here to perform a manual `fold` in
+ // applyOpPatternsGreedily.
+ if (isa<BrOp, ScopeOp, CastOp, UnaryOp>(op))
ops.push_back(op);
});
diff --git a/clang/test/CIR/Transforms/canonicalize.cir b/clang/test/CIR/Transforms/canonicalize.cir
index d61991aef6f01..c0c32627a1096 100644
--- a/clang/test/CIR/Transforms/canonicalize.cir
+++ b/clang/test/CIR/Transforms/canonicalize.cir
@@ -31,6 +31,14 @@ module {
// CHECK-NEXT: cir.return
// CHECK-NEXT: }
+ cir.func @unary_not(%arg0: !cir.bool) -> !cir.bool {
+ %0 = cir.unary(not, %arg0) : !cir.bool, !cir.bool
+ %1 = cir.unary(not, %0) : !cir.bool, !cir.bool
+ cir.return %1 : !cir.bool
+ }
+ // CHECK: cir.func @unary_not(%arg0: !cir.bool) -> !cir.bool
+ // CHECK: cir.return %arg0 : !cir.bool
+
cir.func @cast1(%arg0: !cir.bool) -> !cir.bool {
%0 = cir.cast(bool_to_int, %arg0 : !cir.bool), !s32i
%1 = cir.cast(int_to_bool, %0 : !s32i), !cir.bool
|
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.
I don't have a problem wtih this, but one of the other reviewers/CIR folks ought to doublecheck.
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.
LGTM, the only difference from incubator is the misleading comment, thanks Andy! Will update comment over there as well.
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/18078 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/59/builds/14659 Here is the relevant piece of the build log for the reference
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/88/builds/9410 Here is the relevant piece of the build log for the reference
|
Unary ops had previously been omitted from the list of ops handled in the CIRCanonicalizePass. Although the incubator code doesn't use them directly, the mlir folding code does.
This change enables folding of unary ops by adding them to the list.