Skip to content

Conversation

@anijain2305
Copy link
Contributor

@anijain2305 anijain2305 commented Mar 31, 2020

@yzhliu @kevinthesun

unroll_kw is used for normal conv2d schedules as well. In the case of depthwise conv2d, the input pixels are actually vectorized (not broadcasted as in the case with normal conv2d). This potentially can create opportunities for reusing the data vector across two output pixels. Therefore, adding unroll_kw config option.

In any case, this does not bring perf degradation. It makes search space larger.

Concern - This might require a minor change in Tophub configuration. I can make that change once this PR is merged (or just before).

Copy link
Member

@yzhliu yzhliu left a comment

Choose a reason for hiding this comment

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

looks good to me. feel free to merge @anijain2305 (I don't know whether you want to delay merging until you get the tuning results, I'm fine with either way.)

@anijain2305
Copy link
Contributor Author

Thanks. I will wait for #5200 to get merged and re-run with my PR and add the numbers here. Then, we can merge.

@anijain2305
Copy link
Contributor Author

I tuned mobilenet v2, I dont see any difference in e2e perf, but I see that some depthwise convs are picking up unroll_kw = True (introduced by this PR). Configs for all the depthwise convs are

[('tile_ic', [-1, 32]), ('tile_oc', [-1, 32]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,359
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,1217
[('tile_ic', [-1, 16]), ('tile_oc', [-1, 16]), ('tile_ow', [-1, 4]), ('unroll_kw', False)],None,2378
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,1430
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 14]), ('unroll_kw', True)],None,859
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 14]), ('unroll_kw', True)],None,859
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,859
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', True)],None,85
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', True)],None,85
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', True)],None,85
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 1]), ('unroll_kw', True)],None,85
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 14]), ('unroll_kw', True)],None,1433
[('tile_ic', [-1, 8]), ('tile_oc', [-1, 8]), ('tile_ow', [-1, 14]), ('unroll_kw', True)],None,1433
[('tile_ic', [-1, 4]), ('tile_oc', [-1, 4]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,948
[('tile_ic', [-1, 4]), ('tile_oc', [-1, 4]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,1655
[('tile_ic', [-1, 4]), ('tile_oc', [-1, 4]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,1655
[('tile_ic', [-1, 4]), ('tile_oc', [-1, 4]), ('tile_ow', [-1, 1]), ('unroll_kw', False)],None,1655

@yzhliu yzhliu merged commit 6b840fa into apache:master Apr 3, 2020
@yzhliu
Copy link
Member

yzhliu commented Apr 3, 2020

Thanks @anijain2305

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants