Page MenuHomePhabricator

sjarus (Suraj Sudhir)
User

Projects

User does not belong to any projects.

User Details

User Since
Oct 29 2020, 11:18 AM (11 w, 2 d)

Recent Activity

Mon, Jan 11

sjarus added inline comments to D94247: [MLIR][TOSA] First lowerings from Tosa to Linalg.
Mon, Jan 11, 10:43 AM · Restricted Project

Dec 14 2020

sjarus added a comment to D92581: Add RegionBranchOpInterface to TOSA control flow ops.

At this point the best option appear to be to defer this change. Let's revisit this when the SCF dialect has corresponding tests for this interface. Right now, we aren't able to effectively exercise it, and lack access to precedent material on this interface that we can effectively utilize to construct tests, and don't have familiarity with this domain of MLIR. Thank you for your feedback!

Dec 14 2020, 1:22 PM · Restricted Project
sjarus added inline comments to D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 14 2020, 9:38 AM · Restricted Project
sjarus added a comment to D92581: Add RegionBranchOpInterface to TOSA control flow ops.

Bringing this up again. May I proceed with this ?

Dec 14 2020, 8:04 AM · Restricted Project

Dec 9 2020

sjarus updated the diff for D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 9 2020, 4:54 PM · Restricted Project
sjarus added a comment to D92581: Add RegionBranchOpInterface to TOSA control flow ops.

I've attempted to reconstruct the tests with insights from the sccp tests. While doing so, I restricted this interface to cond_if() alone to simplify this effort. I've also changed the conditional of the op to just a boolean I1 rather than an unnecessary I1Tensor, allowing the check in getSuccessorRegions() to work without modification.

Dec 9 2020, 4:53 PM · Restricted Project

Dec 7 2020

sjarus added inline comments to D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 7 2020, 4:30 PM · Restricted Project
sjarus added inline comments to D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 7 2020, 12:18 PM · Restricted Project

Dec 3 2020

sjarus updated the diff for D92581: Add RegionBranchOpInterface to TOSA control flow ops.

Updated tests.

Dec 3 2020, 6:20 PM · Restricted Project
sjarus added inline comments to D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 3 2020, 6:19 PM · Restricted Project
sjarus updated the diff for D92581: Add RegionBranchOpInterface to TOSA control flow ops.

Incorporated feedback.

Dec 3 2020, 2:36 PM · Restricted Project
sjarus added a comment to D92581: Add RegionBranchOpInterface to TOSA control flow ops.

Incorporated rriddle's feedback and added tests .

Dec 3 2020, 2:35 PM · Restricted Project
sjarus requested review of D92581: Add RegionBranchOpInterface to TOSA control flow ops.
Dec 3 2020, 9:17 AM · Restricted Project

Nov 24 2020

sjarus accepted D92040: [mlir] Add Tosa dialect const folder for tosa.const..
Nov 24 2020, 9:08 AM · Restricted Project

Nov 7 2020

sjarus accepted D91006: NFC: (re-apply) Fix some post-review nits for the Tosa dialect..

Thank you for these cleanup additions!

Nov 7 2020, 9:42 AM · Restricted Project
sjarus added a comment to D90411: TOSA MLIR Dialect.

To suit the flavor of the season, we need a recount here - there was more than 204 since some appear to have disappeared along with deleted files :-)

Nov 7 2020, 9:10 AM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Removed TosaTraits and TosaTypes headers. Will add when implemented.

Nov 7 2020, 7:40 AM · Restricted Project

Nov 6 2020

sjarus updated the diff for D90411: TOSA MLIR Dialect.

Removed extra path in test/Dialect/Tosa .

Nov 6 2020, 11:39 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Updates to feedback from rriddle

Nov 6 2020, 11:24 PM · Restricted Project
sjarus added inline comments to D90411: TOSA MLIR Dialect.
Nov 6 2020, 11:22 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Fixed typo in inlining test. Reran check-mlir to confirm.

Nov 6 2020, 5:12 PM · Restricted Project
sjarus added a comment to D90411: TOSA MLIR Dialect.

I assume you will need me/someone to land this change?

Nov 6 2020, 1:34 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

stellaraccident: Removed extraneous tests as suggested
mehdi_amini: added test decorators.

Nov 6 2020, 1:33 PM · Restricted Project
sjarus added inline comments to D90411: TOSA MLIR Dialect.
Nov 6 2020, 1:32 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Incorporates feedback from stellaraccident, rriddle, mehdi_amini

  • Cleanup nits and mechanicals
  • Removed extraneous tests
  • Quantization Builders in C++ now
  • Aligned pass invocation with current approach
  • 80-col alignment
Nov 6 2020, 11:06 AM · Restricted Project
sjarus added inline comments to D90411: TOSA MLIR Dialect.
Nov 6 2020, 11:04 AM · Restricted Project

Nov 5 2020

sjarus added a comment to D90411: TOSA MLIR Dialect.

We'll work on these nits and upload a new patch soon. Since we're new to this, if the next patch looks ok, then please do land it on our behalf as an act of approval.

Nov 5 2020, 12:03 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Updates contain:

  • Aesthetic cleanups, usage of better MLIR constructs
  • Unit tests
  • WIP rationale document
Nov 5 2020, 7:52 AM · Restricted Project
sjarus added inline comments to D90411: TOSA MLIR Dialect.
Nov 5 2020, 7:50 AM · Restricted Project

Oct 30 2020

sjarus updated the diff for D90411: TOSA MLIR Dialect.

Missed one file update in prior commit

Oct 30 2020, 4:26 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Updated to incorporate comments from mehdi_amini and stellaraccident.

Oct 30 2020, 4:02 PM · Restricted Project
sjarus added inline comments to D90411: TOSA MLIR Dialect.
Oct 30 2020, 4:01 PM · Restricted Project

Oct 29 2020

sjarus added a comment to D90411: TOSA MLIR Dialect.

Sure, but can you point out exactly where is it tested in this patch? My basic mental model is: if I change the behavior of any line of code, will it fails when running ninja check-mlir?
Again: the benefit of splitting this is that we can land patches more incrementally, and individual important pieces, like moveOutOfLoop and the LoopLikeOpInterface can be tested in isolation with the patch that land them.

These are both good questions. It seems they could both be resolved if we add tests to exercise these in mlir/test/Dialect/Tosa . If so, could we please do that as a follow on effort ?

I'm happy to review the code right now, but I'd also like the tests to get in with the code and not as a follow-up. In general we don't land patches without tests as far as I know.

Oct 29 2020, 6:23 PM · Restricted Project
sjarus added a comment to D90411: TOSA MLIR Dialect.

In the context of this patch, I just randomly sampled some functions involved here and couldn't find an obvious caller? For example buildQTypeAttrFromMinMax or all the computeMultiplierAndShift*

Oct 29 2020, 5:34 PM · Restricted Project
sjarus updated the diff for D90411: TOSA MLIR Dialect.

Updates to incorporate feedback from bondhugula and mehdi_amini .

  • Comment formatting
  • camelCase vars
  • Removed extraneous computeMultiplierAndShift* functions
Oct 29 2020, 5:26 PM · Restricted Project
sjarus added a comment to D90411: TOSA MLIR Dialect.

Thanks, really nice!
Can you split out some pieces so we can focus on the dialect itself mostly?
I'm thinking about the QuantUtils parts (which aren't used/tested in this patch anyway) and a few things like the TosaMakeBroadcastable pass, or the TosaOpInterface or the moveOutOfLoop() method and similarly things which aren't clearly needed right now.

Oct 29 2020, 2:37 PM · Restricted Project
sjarus updated the summary of D90411: TOSA MLIR Dialect.
Oct 29 2020, 11:28 AM · Restricted Project
sjarus requested review of D90411: TOSA MLIR Dialect.
Oct 29 2020, 11:27 AM · Restricted Project