Automated commit created by applying diff 489654
Phabricator-ID: PHID-HMBT-sx4k6i2amcuosqdga4sn
Review-ID: D141880
Differential D141900
Add support for processing Ops with empty region to the CSE Ding on Jan 17 2023, 12:24 AM. Authored by
Details
Automated commit created by applying diff 489654 Phabricator-ID: PHID-HMBT-sx4k6i2amcuosqdga4sn
Diff Detail
Event TimelineComment Actions Thanks! This was the fix I had in mind. This is where the tests for ops with region are https://github.com/llvm/llvm-project/blob/d2f136920b9247a9e5874d4d3a00a880db6e2827/mlir/test/Transforms/cse.mlir#L327 This is the test op definition https://github.com/llvm/llvm-project/blob/d2f136920b9247a9e5874d4d3a00a880db6e2827/mlir/test/lib/Dialect/Test/TestOps.td#L3141 . You might need to add a new op, or change the op definition to test your case.
Comment Actions I found that test.any_cond Op could have empty region, maybe we can use it for testing? Just like this func.func @cse_ops_with_empty_region() -> (i32, i32) { %0 = "test.any_cond"() ({ }) : () -> i32 %1 = "test.any_cond"() ({ }) : () -> i32 return %0, %1 : i32, i32 } the IR after CSE Pass will be func.func @cse_single_block_ops_identical_bodies() -> (i32, i32) { %0 = "test.any_cond"() ({ }) : () -> i32 return %0, %0 : i32, i32 }
Comment Actions This commit is conflict with the latest commit which support multiple region CSE, I will resolve these conflicts this weekend. Comment Actions @mravishankar, I found that commit aa4e54f2f444c266310105fccbdbb07fc71894da has fixed this issue when trying to support CSE ops with multiple regions. Should I just push this commit with the new test I added in cse.mlir? |
Please pull these out of the namespace, static functions should be top level.