-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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.
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
[ARM] Support NCHWc alter layout in the fallback mode #10724
Conversation
@@ -101,9 +102,6 @@ def _alter_conv2d_layout(attrs, inputs, tinfos, out_type): | |||
# we then assume it's not necessary to alter this op. | |||
return None | |||
cfg = dispatch_ctx.query(target, workload) | |||
if cfg.is_fallback: # if is fallback, clear query cache and return None | |||
autotvm.task.clear_fallback_cache(target, workload) | |||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope it's safe to remove this. The x86 counterpart doesn't have thing like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are you testing it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I can pass the CI with this change, I assume it is safe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mousius - could you please look at this given you've recently been turning on topi tests on aarch64 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to remove this code because it always makes alter_layout
nop in the fallback mode. In contrast, in the x86 schedule, alter_layout
always fires.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI has passed. I found that Giuseppe's im2col based conv2d implementation can fail to tensorize in the fallback mode, so I partially restored the fallback return path above.
8acb02c
to
a05d81f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good except for a few small changes.
a05d81f
to
0beacdf
Compare
@@ -149,5 +151,49 @@ def extract_task_qbert(): | |||
assert "vnni" in annotations["schedule_rule"] | |||
|
|||
|
|||
def extract_task_arm_conv2d_nchwc(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this runs @masahi? There's no test_
prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops thanks for pointing this out, I added a fix in #10773
* [ARM] Support NCHWc alter layout in the fallback mode * remove fallback path * add test * fixed int32_lanes and add channel check * fixed schedule dispatch bug * add workaround fallback path for NHWC im2col based GEMM schedule * int32_lanes=4 by default * typo * update test
…ts matrix for arm_cpu NHWC quantized conv2d Fixed arm_cpu strategy bug which was causing tensorization errors when using the `AlterOpLayout` pass for the quantized NHWC conv2d schedules, as discovered in apache#10724. Therefore, we can now also enable the usage of `AlterOpLayout` for these schedules in order to transform the weight matrix at compile time, instead of runtime as before. I also modified the padding in `Conv2DGemmWeightTransformRel` and `interleave_transpose_weights` to reflect the changes made in apache#13669 and updated the AlterOpLayout tests accordingly.
…ts matrix for arm_cpu NHWC quantized conv2d (#15584) Fixed arm_cpu strategy bug which was causing tensorization errors when using the `AlterOpLayout` pass for the quantized NHWC conv2d schedules, as discovered in #10724. Therefore, we can now also enable the usage of `AlterOpLayout` for these schedules in order to transform the weight matrix at compile time, instead of runtime as before. I also modified the padding in `Conv2DGemmWeightTransformRel` and `interleave_transpose_weights` to reflect the changes made in #13669 and updated the AlterOpLayout tests accordingly.
I want to extract tuning tasks for ARM int8 tensorization. The current
alter_layout
code intopi/arm_cpu
doesn't fire in the fallback mode, which has been fixed in the PR.@tkonolige @comaniac