Page MenuHomePhabricator

sumesh13 (Sumesh Udayakumaran)
User

Projects

User does not belong to any projects.

User Details

User Since
Apr 1 2021, 9:31 AM (17 w, 4 d)

Recent Activity

Yesterday

sumesh13 updated the summary of D106473: [NFC][MLIR] Split large fusion test file into two test files.
Sun, Aug 1, 3:13 PM · Restricted Project
sumesh13 updated the diff for D106473: [NFC][MLIR] Split large fusion test file into two test files.

4 -way split that brings down the time per test from 5.5 seconds on a 16 core machine to about 1.5 second. The last file has some more room to grow.

Sun, Aug 1, 3:12 PM · Restricted Project

Thu, Jul 22

sumesh13 added a comment to D106473: [NFC][MLIR] Split large fusion test file into two test files.

Here is using time on a 16 core machine

Thu, Jul 22, 12:23 PM · Restricted Project

Wed, Jul 21

sumesh13 added a comment to D106473: [NFC][MLIR] Split large fusion test file into two test files.

mlir/test/transforms/loop-fusion.mlir is too big and is split into mlir/test/transforms/loop-fusion.mlir and mlir/test/transforms/loop-fusion-2.mlir

On which basis? Don't you think having _two similar names_ make confusion who don't know the context of test splitting.

This test case file has been slowing down check-mlir for everyone. I won't be surprised if this is the one that runs the longest across all test cases in the codebase. It has to be split. Open to a better/logical way of partitioning (for eg. sibling fusion vs producer consumer fusion) but they both might just be intertwined to separate in a balanced way. It is common for large files to be split this way - both sources and test cases. For eg. in the TF/MLIR repo, tf_ops.cc became too large and they went with something like t_ops_a_m.cc and tf_ops_n_z.cc to cut down the 60+s compile time.

The main problem for me is that this split feels rather arbitrary. The original file still has >3000 lines. Has any benchmarking gone into determining the split point/number of partitions/etc?

Wed, Jul 21, 8:43 PM · Restricted Project
sumesh13 requested review of D106473: [NFC][MLIR] Split large fusion test file into two test files.
Wed, Jul 21, 12:07 PM · Restricted Project

Thu, Jul 15

sumesh13 committed rGada580863f89: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused… (authored by sumesh13).
[mlir] Enable cleanup of single iteration reduction loops being sibling-fused…
Thu, Jul 15, 2:08 PM
sumesh13 closed D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Thu, Jul 15, 2:07 PM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

One asan fix

Thu, Jul 15, 10:38 AM · Restricted Project

Wed, Jul 14

sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Fix a clang-format issue

Wed, Jul 14, 11:59 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

FIx a clang-format issue

Wed, Jul 14, 11:58 AM · Restricted Project
sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Thanks @bondhugula @vinayaka-polymage for the review and helpful comments!!

Wed, Jul 14, 11:44 AM · Restricted Project

Mon, Jul 12

sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

isReductionLoop->isLoopParallelAndContainsReductionLoop

Mon, Jul 12, 10:29 PM · Restricted Project
sumesh13 added inline comments to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Mon, Jul 12, 7:18 PM · Restricted Project

Sun, Jul 11

sumesh13 added inline comments to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Sun, Jul 11, 10:35 PM · Restricted Project

Thu, Jul 8

sumesh13 added inline comments to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Thu, Jul 8, 11:02 PM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Adrress review comments

Thu, Jul 8, 10:58 PM · Restricted Project

Tue, Jul 6

sumesh13 added inline comments to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Tue, Jul 6, 12:46 PM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address comments

  • Reflow all coments
    • Additional argument to promoteSingleIterReduction to control when operations are moved out
Tue, Jul 6, 12:44 PM · Restricted Project

Jun 28 2021

sumesh13 added inline comments to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Jun 28 2021, 11:58 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Add an extra clarification for when the promotion API can be safely used.

Jun 28 2021, 11:57 AM · Restricted Project
sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

@vinayaka-polymage Thanks for additional comments. Can you pls check if my clarifications address your concerns?

Jun 28 2021, 11:50 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address comments

Jun 28 2021, 11:42 AM · Restricted Project

Jun 24 2021

sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Reflow a comment

Jun 24 2021, 11:40 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Reflow a comment

Jun 24 2021, 11:39 AM · Restricted Project
sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

@sumesh13 Could you also please mention whether some of the methods were just moved without any change? That's useful in the commit summary.

Jun 24 2021, 10:41 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address code review comments including rename API to replaceForOpWithNewYields

Jun 24 2021, 10:37 AM · Restricted Project

Jun 23 2021

sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Thansk for the additional comments.

Jun 23 2021, 12:15 PM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address review comments and add check to only do cleanup if the loop trip counts match.

Jun 23 2021, 12:12 PM · Restricted Project

Jun 21 2021

sumesh13 added a comment to D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Thanks for the feedback. I have addressed the review comments. Please take a look.

Jun 21 2021, 10:17 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address Code review comments

Jun 21 2021, 10:15 AM · Restricted Project
sumesh13 updated the diff for D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .

Address Code review comments

Jun 21 2021, 9:59 AM · Restricted Project

Jun 14 2021

sumesh13 requested review of D104249: [mlir] Enable cleanup of single iteration reduction loops being sibling-fused maximally .
Jun 14 2021, 11:22 AM · Restricted Project

Apr 13 2021

sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

@bondhugula , @vinayaka-polymage thanks for the review. @sgrechanik Can you pls help land this

Apr 13 2021, 11:47 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Address comment on simplifying condition to add ops with users.

Apr 13 2021, 10:46 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Simplify condition to collect ops with users.

Apr 13 2021, 10:45 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Gentle reminder..also pls suggest if it would be better to collect all the opsWithUsers separately as i was doing in a previous revision. For nested ifs, looks like otherwise you may not be collecting all the opsWithUsers by just collecting at the top level and leaves.

Apr 13 2021, 10:23 AM · Restricted Project

Apr 12 2021

sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Adding back a change from previous revision that was approved and some minor cleanup.

Apr 12 2021, 10:15 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Revert to earlier revision that was approved and some minor cleanup of comments.

Apr 12 2021, 10:13 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

@vinayaka-polymage Just noticed that you accepted a previous revision with comment on handling of ifops. Considering ifops, would you think it might be helpful to just collect all the ops like I was doing in a previous revision ?

Apr 12 2021, 10:10 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Address comments

Apr 12 2021, 10:03 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Mark review comments as done and undo a typo that had crept in

Apr 12 2021, 9:16 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Undo a change w.r.t to the base that had crept in

Apr 12 2021, 9:15 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

@vinayaka-polymage Thanks for the comments. Pls take a look

Apr 12 2021, 9:01 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Address review comments on code reorg

Apr 12 2021, 9:00 AM · Restricted Project

Apr 11 2021

sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

I have addresses the comments. Pls take a look.

Apr 11 2021, 11:19 AM · Restricted Project
sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Thank you for bringing up this issue in LICM, it is an important one. I have a few comments on the fix:

  1. Inherent issue is definedOps ( going to be renamed to opsWithUsers) is being populated only under some conditions, and does not actually do the job of collecting all ops that define an SSA value in that block.
  2. Even after this fix, only AffineForOps will be added to the list, but there are other cases like AffineDma... ops, AffineIfOps that are actually NOT code invariant etc. which will still cause similar problems.

So, I suggest that you can implement a generic fix for this inherent issue, where any op that defines an SSA value can be added to the above list, and this is decoupled from the checks like 'isOpLoopInvariant`. This will make the check more robust, and relevant test cases can also be added.

Apr 11 2021, 11:17 AM · Restricted Project
sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.

Rework based on comments

  • Separate the part that identifies operations that have users and use it in the later phase.
Apr 11 2021, 11:15 AM · Restricted Project
sumesh13 retitled D99761: [mlir] Prevent operations with users from being hoisted from [mlir] Prevent AffineFor users from being hoisted to [mlir] Prevent operations with users from being hoisted.
Apr 11 2021, 11:12 AM · Restricted Project

Apr 7 2021

sumesh13 added a comment to D99761: [mlir] Prevent operations with users from being hoisted.

Gentle ping

Apr 7 2021, 8:27 PM · Restricted Project

Apr 5 2021

sumesh13 added inline comments to D99761: [mlir] Prevent operations with users from being hoisted.
Apr 5 2021, 1:42 PM · Restricted Project

Apr 2 2021

sumesh13 updated the diff for D99761: [mlir] Prevent operations with users from being hoisted.
  1. Address code review comments including renaming definedOps to opsWithusers
Apr 2 2021, 8:37 AM · Restricted Project

Apr 1 2021

sumesh13 added a reviewer for D99761: [mlir] Prevent operations with users from being hoisted: bondhugula.
Apr 1 2021, 1:47 PM · Restricted Project
sumesh13 requested review of D99761: [mlir] Prevent operations with users from being hoisted.
Apr 1 2021, 12:37 PM · Restricted Project