[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
andykaylor wrote: @bcardosolopes Do you have any comments on this before I merge it? https://github.com/llvm/llvm-project/pull/130861 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
andykaylor wrote:
I don't think it should. The general intent is for passes to perform a specific
purpose and rely on layering of passes to clean things up. Because the
canonicalization pass needs to remove useless blocks anyway, there is no
advantage to also doing that in the flatten pass.
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
erichkeane wrote:
> These checks are the same checks that are present in this test in the
> incubator (except for the fact that we're not abbreviating the int types
> upstream yet). The test is verifying the flattening pass in isolation, and
> since the flattening pass doesn't remove unneeded blocks, the output will
> stay as it is here.
My question was really "should it?" to the "the flattening pass doesn't remove
unneeded blocks". But TBH my understanding of what we should expect out of
individual passes, or how passes are designed is lacking.
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
andykaylor wrote:
These checks are the same checks that are present in this test in the incubator
(except for the fact that we're not abbreviating the int types upstream yet).
The test is verifying the flattening pass in isolation, and since the
flattening pass doesn't remove unneeded blocks, the output will stay as it is
here.
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
erichkeane wrote:
This here is a touch awkward, right? I know we did a bit of work to remove
'empty' blocks in the pass. I wonder if a future version of this pass should
be checking the 'exit' of a block to see if it is a single-out (that is, no
decisions being done, just a single `br`, and merge the two. It seems to fit
in well with the flatten.
That is, this function here seems like it should/could be a single block, right?
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
https://github.com/erichkeane approved this pull request. I don't have concerns, but am not the best person whose approval should count here. I have questions/comments on the scope of the flatten pass (that is, what 'while we are here' type stuff it should do), but I'm again not really an authority on these. https://github.com/llvm/llvm-project/pull/130861 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
dkolsen-pgi wrote:
In the patch I am currently working on I am adding the Canonicalization pass,
which merges/removes useless blocks. So this CIR will look better after that
is done. (At least I think it will. The canonicalization pass runs before the
flattening pass. I can't promise that it will also run after.)
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
https://github.com/erichkeane edited https://github.com/llvm/llvm-project/pull/130861 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [CIR] Add transform test for cir-flatten-cfg (PR #130861)
@@ -0,0 +1,58 @@
+// RUN: cir-opt %s -cir-flatten-cfg -o - | FileCheck %s
+
+module {
+ cir.func @foo() {
+cir.scope {
+ %0 = cir.alloca !cir.int, !cir.ptr>, ["a", init]
{alignment = 4 : i64}
+ %1 = cir.const #cir.int<4> : !cir.int
+ cir.store %1, %0 : !cir.int, !cir.ptr>
+}
+cir.return
+ }
+// CHECK: cir.func @foo() {
+// CHECK:cir.br ^bb1
+// CHECK: ^bb1: // pred: ^bb0
+// CHECK:%0 = cir.alloca !cir.int, !cir.ptr>, ["a",
init] {alignment = 4 : i64}
+// CHECK:%1 = cir.const #cir.int<4> : !cir.int
+// CHECK:cir.store %1, %0 : !cir.int, !cir.ptr>
+// CHECK:cir.br ^bb2
+// CHECK: ^bb2: // pred: ^bb1
+// CHECK:cir.return
+// CHECK: }
+
+ // Should drop empty scopes.
+ cir.func @empty_scope() {
+cir.scope {
+}
+cir.return
+ }
+// CHECK: cir.func @empty_scope() {
+// CHECK:cir.return
+// CHECK: }
+
+ cir.func @scope_with_return() -> !cir.int {
+%0 = cir.alloca !cir.int, !cir.ptr>, ["__retval"]
{alignment = 4 : i64}
+cir.scope {
+ %2 = cir.const #cir.int<0> : !cir.int
+ cir.store %2, %0 : !cir.int, !cir.ptr>
+ %3 = cir.load %0 : !cir.ptr>, !cir.int
+ cir.return %3 : !cir.int
+}
+%1 = cir.load %0 : !cir.ptr>, !cir.int
+cir.return %1 : !cir.int
+ }
+
+// CHECK: cir.func @scope_with_return() -> !cir.int {
+// CHECK:%0 = cir.alloca !cir.int, !cir.ptr>,
["__retval"] {alignment = 4 : i64}
+// CHECK:cir.br ^bb1
erichkeane wrote:
This is another block that it seems to me could be removed/merged with `bb1`.
I guess there are later passes that do these sorts of merges? Also, removing
`bb2` (which is an obviously `dead` block).
https://github.com/llvm/llvm-project/pull/130861
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
