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

[TOPI][x86] Cascade lake support. #4123

Merged
merged 3 commits into from
Oct 17, 2019
Merged

[TOPI][x86] Cascade lake support. #4123

merged 3 commits into from
Oct 17, 2019

Conversation

anijain2305
Copy link
Contributor

As Title. Enables Intel VNNI instructions for Int8 quantized networks.

@anijain2305 anijain2305 force-pushed the cascade branch 2 times, most recently from 29070d0 to 1d96d39 Compare October 14, 2019 20:34
tmoreau89
tmoreau89 approved these changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Thanks @anijain2305 for the changes!

tmoreau89
tmoreau89 reviewed Oct 14, 2019
View reviewed changes
Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Nit: why do we have the intrinsics named {dot_16x1x16_uint8_int8_int32_skylake, dot_16x1x16_uint8_int8_int32_vnni} instead of {dot_16x1x16_uint8_int8_int32_skylake, dot_16x1x16_uint8_int8_int32_cascadelake}?

@anijain2305
Copy link
Contributor Author

Nit: why do we have the intrinsics named {dot_16x1x16_uint8_int8_int32_skylake, dot_16x1x16_uint8_int8_int32_vnni} instead of {dot_16x1x16_uint8_int8_int32_skylake, dot_16x1x16_uint8_int8_int32_cascadelake}?

Will make it more consistent. Thanks for the suggestion :)

@anijain2305 anijain2305 force-pushed the cascade branch 5 times, most recently from 719a583 to d58aead Compare October 15, 2019 19:14
tmoreau89
tmoreau89 reviewed Oct 15, 2019
View reviewed changes
tests/python/relay/test_op_level2.py Outdated
assert False, "Target should be Skylake or Cascadelake"

# compile conv2d for x86 (skylake, cascadelake) and test assembly contains *pmadd* instructions
# targets = ["llvm -mcpu=skylake-avx512", "llvm -mcpu=cascadelake"]
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this commented out? do we not want to test the cascadelake target?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, Let me add WIP tag. Jenkins is failing w/o any debug prints. So I am trying to narrow down the issue. I will ping you once I identify the issue.

tmoreau89
tmoreau89 reviewed Oct 15, 2019
View reviewed changes
Copy link
Contributor

@tmoreau89 tmoreau89 left a comment

Choose a reason for hiding this comment

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

Added one more comment. Thanks for the changes!

@anijain2305 anijain2305 changed the title [TOPI][x86] Cascade lake support. [TOPI][x86] WIP - Cascade lake support. Oct 15, 2019
soiferj
soiferj reviewed Oct 15, 2019
View reviewed changes
topi/python/topi/x86/tensor_intrin.py Outdated
target = tvm.target.current_target()
intel_device_type = None
for opt in target.options:
if opt == '-mcpu=skylake-avx512':
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a helper function to get CPU type from target? If not, I think that adding it would make lines like this a lot cleaner, and a lot friendlier to new developers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. Let me see if I can get that parsing a part of target data structure.

@anijain2305 anijain2305 force-pushed the cascade branch 6 times, most recently from a2194b9 to a0d7041 Compare October 16, 2019 18:37
@anijain2305
Copy link
Contributor Author

@tmoreau89 @soiferj I am done now. Guarded the tests to work with only >=8 LLVM version. That resolved the CI error.

Please review again and let me know if you have more comments.

@anijain2305 anijain2305 changed the title [TOPI][x86] WIP - Cascade lake support. [TOPI][x86] Cascade lake support. Oct 16, 2019
soiferj
soiferj suggested changes Oct 16, 2019
View reviewed changes
python/tvm/target.py Outdated
@@ -521,6 +521,28 @@ def create(target_str):
return _api_internal._TargetFromString(target_str)


def get_mcpu(target):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to add this as a property under the Target class? Like libs

Copy link
Contributor Author

@anijain2305 anijain2305 Oct 16, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Moved it. Please review again :)

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@tmoreau89 tmoreau89 merged commit 972f019 into apache:master Oct 17, 2019
anijain2305 added a commit to anijain2305/tvm that referenced this pull request Oct 17, 2019
@anijain2305
[TOPI][x86] Cascade lake support. (apache#4123)
a2415f4 
* [TOPI][x86] Cascade lake support.

* Jenkins test debug 1.

* Testing cascade lake alone.
wweic pushed a commit to neo-ai/tvm that referenced this pull request Oct 18, 2019
@anijain2305 @wweic
[TOPI][x86] Cascade lake support. (apache#4123)
2f9be03 
* [TOPI][x86] Cascade lake support.

* Jenkins test debug 1.

* Testing cascade lake alone.
petrex added a commit to petrex/tvm that referenced this pull request Oct 29, 2019
@petrex
Merge branch 'master' into benchmark
0e9a464 
* master: (51 commits)
  [QNN][TFLite] Parsing QNN Add op. Adding MobilenetV2. (apache#4142)
  [CI] Pin NNPack pthreadtools version (apache#4152)
  Fix typo (apache#4144)
  [Relay][Frontend][TF] Add tensor array ops (apache#3798)
  [relay][vm] Separate VM runtime with executable (apache#4100)
  [PATCH] Fix undefined __floatdihf in libtvmruntime.so on aarch64. (apache#4119)
  [DOCKER] Pin torchvision==0.4.1 (apache#4140)
  [TOPI][x86] Cascade lake support. (apache#4123)
  [Relay] Improve build error when no lowered funcs are produced (apache#4132)
  [RUNTIME] Refactor object python FFI to new protocol. (apache#4128)
  Update PULL_REQUEST_TEMPLATE.md
  Adding support for dequantizing from int32 to float32. (apache#4130)
  [Relay][Training] Add and fix gradients (apache#4126)
  [QNN] Change default rouning to UPWARD. (apache#4131)
  Fix infer type of kernel in dense. (apache#4125)
  [Relay][AlterOpLayout] NHWC to NCHWc pad operator. (apache#4103)
  [ARITH] Fix lowering of floormod(x, y) != 0 (apache#4127)
  [RFC][RUNTIME] Introduce new object protocol. (apache#4115)
  [Relay][Topi] Disable conv NHWC pack int8. (apache#4038)
  Update task_cpp_unittest.sh
  ...
@tqchen tqchen mentioned this pull request Nov 8, 2019
Closed
@anijain2305 anijain2305 deleted the cascade branch November 13, 2019 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tmoreau89 tmoreau89 tmoreau89 approved these changes

@soiferj soiferj soiferj approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@anijain2305 @soiferj @tmoreau89