Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[TIR][USMP] Add a parallel to serial for loop converter pass #8469

Merged
merged 5 commits into from
Oct 12, 2021
Merged

[TIR][USMP] Add a parallel to serial for loop converter pass #8469

merged 5 commits into from
Oct 12, 2021

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Jul 14, 2021 •
edited
Loading

This is an optional pass to convert all parallel for loops in TIR to serial ones for different reasons such as executor does not support parallel launch of for loops (e.g., AoT) or allocating space for parallel for loops might not be desired.

@manupak manupak mentioned this pull request Jul 14, 2021
Closed
21 tasks
@tqchen
Copy link
Member

tqchen commented Jul 14, 2021

given the particular pass can be applied to any TIR Module, we might be able to just put it under tir namespace withour introducing the usmp namespace.

@junrushao
Copy link
Member

Thanks for the contribution! Just curious, why we do such a pass instead of removing sch.parallel in scheduling?

@manupak
Copy link
Contributor Author

manupak commented Jul 19, 2021

Hi @tqchen ,
Yes make sense, will do.

Hi @junrushao1994 ,
I think TOPI and other places define that as a property of the op implementation/scheduling. However, this is used as post-scheduling transformation to force the parallel loops to be serial to reduce memory and/or explicitly depict what the intended executor / backend supports.

manupak added 5 commits August 6, 2021 16:55
@manupak
[TIR][USMP] Add a parallel to serial for loop converter pass
bfcac8c 
This is an optional pass to convert all parallel for loops in TIR
to serial ones for different reasons such as executor
does not support parallel launch of for loops (e.g., AoT)
or allocating space for parallel for loops might not
be desired.

* Additionally adding FFI scaffolding for USMP

Change-Id: Id5e8ccb90140d2d3ae113b20a3ca152a54497c45
@manupak
[TIR][USMP] Add a parallel to serial for loop converter pass
2b9f5d7 
* remove unused import

Change-Id: I29d5fdec92120418596f9dba1d6630f65620a603
@manupak
[TIR][USMP] Add a parallel to serial for loop converter pass
3dac799 
*moved the pass to tir namespace

Change-Id: I74720ca2f566066b3a4f22f504d8f0f684c99dc2
@manupak
[TIR][USMP] Add a parallel to serial for loop converter pass
a424b9a 
* fixed docstring

Change-Id: I73bb9867fe2ed6a86f65666493c5c6e3edf87b49
@manupak
[TIR][USMP] Add a parallel to serial for loop converter pass
26571f8 
* fixed mypy lint error

Change-Id: I226ef27d5536674fbe4b2d2c6ff47b8cb3b41431
@manupak manupak force-pushed the usmp_parallel_for_to_serial_for_converter branch from 338e51a to 26571f8 Compare August 6, 2021 16:19
areusch
areusch reviewed Sep 13, 2021
View reviewed changes
tests/python/unittest/test_tir_transform_convert_for_loops_serial.py

def test_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2():
primfunc = fused_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2
mod = tvm.IRModule.from_expr(primfunc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

want to assert that you find at least one kParallel for loop in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to ? I mean its written in the test.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's consider this blocked on testing infrastructure. a common pattern in tests is for the data to be re-used in multiple tests and then lose the "why" behind the test. that's where my request is coming from.

manupak
manupak commented Oct 7, 2021
View reviewed changes
Copy link
Contributor Author

@manupak manupak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tqchen @areusch @junrushao1994
Can we progress this one ?

tests/python/unittest/test_tir_transform_convert_for_loops_serial.py

def test_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2():
primfunc = fused_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2
mod = tvm.IRModule.from_expr(primfunc)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to ? I mean its written in the test.

areusch
areusch approved these changes Oct 12, 2021
View reviewed changes
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @manupa-arm! cc @mbs-octoml

tests/python/unittest/test_tir_transform_convert_for_loops_serial.py

def test_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2():
primfunc = fused_nn_conv2d_add_fixed_point_multiply_clip_cast_cast_2
mod = tvm.IRModule.from_expr(primfunc)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's consider this blocked on testing infrastructure. a common pattern in tests is for the data to be re-used in multiple tests and then lose the "why" behind the test. that's where my request is coming from.

@areusch areusch merged commit a7cf317 into apache:main Oct 12, 2021
Mousius added a commit to Mousius/tvm that referenced this pull request Oct 12, 2021
@Mousius
Fix USMP parallel to serial loop transform test
93e8e6a 
Caused by apache#8469 being stale on merge when apache#9115 had changed the namespace for `tvm.script`.
@Mousius Mousius mentioned this pull request Oct 12, 2021
Merged
mbaret pushed a commit that referenced this pull request Oct 12, 2021
@Mousius
Fix USMP parallel to serial loop transform test (#9254)
f4922bc 
Caused by #8469 being stale on merge when #9115 had changed the namespace for `tvm.script`.
masahi pushed a commit to Laurawly/tvm-1 that referenced this pull request Oct 14, 2021
@manupak @masahi
[TIR][USMP] Add a parallel to serial for loop converter pass (apache#…
9dd4ec3 
…8469)

* [TIR][USMP] Add a parallel to serial for loop converter pass

This is an optional pass to convert all parallel for loops in TIR
to serial ones for different reasons such as executor
does not support parallel launch of for loops (e.g., AoT)
or allocating space for parallel for loops might not
be desired.

* Additionally adding FFI scaffolding for USMP

Change-Id: Id5e8ccb90140d2d3ae113b20a3ca152a54497c45

* [TIR][USMP] Add a parallel to serial for loop converter pass

* remove unused import

Change-Id: I29d5fdec92120418596f9dba1d6630f65620a603

* [TIR][USMP] Add a parallel to serial for loop converter pass

*moved the pass to tir namespace

Change-Id: I74720ca2f566066b3a4f22f504d8f0f684c99dc2

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed docstring

Change-Id: I73bb9867fe2ed6a86f65666493c5c6e3edf87b49

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed mypy lint error

Change-Id: I226ef27d5536674fbe4b2d2c6ff47b8cb3b41431
masahi pushed a commit to Laurawly/tvm-1 that referenced this pull request Oct 14, 2021
@Mousius @masahi
Fix USMP parallel to serial loop transform test (apache#9254)
ed28667 
Caused by apache#8469 being stale on merge when apache#9115 had changed the namespace for `tvm.script`.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@manupak @ylc
[TIR][USMP] Add a parallel to serial for loop converter pass (apache#…
810d236 
…8469)

* [TIR][USMP] Add a parallel to serial for loop converter pass

This is an optional pass to convert all parallel for loops in TIR
to serial ones for different reasons such as executor
does not support parallel launch of for loops (e.g., AoT)
or allocating space for parallel for loops might not
be desired.

* Additionally adding FFI scaffolding for USMP

Change-Id: Id5e8ccb90140d2d3ae113b20a3ca152a54497c45

* [TIR][USMP] Add a parallel to serial for loop converter pass

* remove unused import

Change-Id: I29d5fdec92120418596f9dba1d6630f65620a603

* [TIR][USMP] Add a parallel to serial for loop converter pass

*moved the pass to tir namespace

Change-Id: I74720ca2f566066b3a4f22f504d8f0f684c99dc2

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed docstring

Change-Id: I73bb9867fe2ed6a86f65666493c5c6e3edf87b49

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed mypy lint error

Change-Id: I226ef27d5536674fbe4b2d2c6ff47b8cb3b41431
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 7, 2022
@Mousius @ylc
Fix USMP parallel to serial loop transform test (apache#9254)
695de94 
Caused by apache#8469 being stale on merge when apache#9115 had changed the namespace for `tvm.script`.
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@manupak @ylc
[TIR][USMP] Add a parallel to serial for loop converter pass (apache#…
95e7a8f 
…8469)

* [TIR][USMP] Add a parallel to serial for loop converter pass

This is an optional pass to convert all parallel for loops in TIR
to serial ones for different reasons such as executor
does not support parallel launch of for loops (e.g., AoT)
or allocating space for parallel for loops might not
be desired.

* Additionally adding FFI scaffolding for USMP

Change-Id: Id5e8ccb90140d2d3ae113b20a3ca152a54497c45

* [TIR][USMP] Add a parallel to serial for loop converter pass

* remove unused import

Change-Id: I29d5fdec92120418596f9dba1d6630f65620a603

* [TIR][USMP] Add a parallel to serial for loop converter pass

*moved the pass to tir namespace

Change-Id: I74720ca2f566066b3a4f22f504d8f0f684c99dc2

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed docstring

Change-Id: I73bb9867fe2ed6a86f65666493c5c6e3edf87b49

* [TIR][USMP] Add a parallel to serial for loop converter pass

* fixed mypy lint error

Change-Id: I226ef27d5536674fbe4b2d2c6ff47b8cb3b41431
ylc pushed a commit to ylc/tvm that referenced this pull request Jan 13, 2022
@Mousius @ylc
Fix USMP parallel to serial loop transform test (apache#9254)
f48272d 
Caused by apache#8469 being stale on merge when apache#9115 had changed the namespace for `tvm.script`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@areusch areusch areusch approved these changes

@comaniac comaniac Awaiting requested review from comaniac

@jroesch jroesch Awaiting requested review from jroesch

@junrushao junrushao Awaiting requested review from junrushao

@kparzysz-quic kparzysz-quic Awaiting requested review from kparzysz-quic

@masahi masahi Awaiting requested review from masahi

@merrymercy merrymercy Awaiting requested review from merrymercy

@tqchen tqchen Awaiting requested review from tqchen

@vinx13 vinx13 Awaiting requested review from vinx13

@yzhliu yzhliu Awaiting requested review from yzhliu

@ZihengJiang ZihengJiang Awaiting requested review from ZihengJiang

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@manupak @tqchen @junrushao @areusch