Build and test TVM under minimal configuration #12178

gigiblender · 2022-07-25T18:21:53Z

This PR builds and tests TVM (running the CPP and unittests) under minimal configuration with some debug flags enabled:

USE_RELAY_DEBUG=ON in TVM
-Wp,-D_GLIBCXX_ASSERTIONS in TVM
-DLLVM_ENABLE_ASSERTIONS=ON in LLVM

It also adds this configuration to the CI.

tests/python/unittest/test_meta_schedule_task_scheduler.py::test_meta_schedule_task_scheduler_multiple_gradient_based results in an array OOB access and a segfault due to D_GLIBCXX_ASSERTIONS. I disable this test for now and will open an issue to solve it ASAP.

It should fix #11932 and address this discussion.

gigiblender · 2022-07-25T19:28:52Z

@areusch @driazati

driazati

Thanks for tackling this! It's been on the issue list for quiet a while, things mostly lgtm. Can you also follow up to add an ECR repo for ci_minimal as well? https://github.com/tlc-pack/ci/blob/main/terraform/vars/tvm-ci-prod.auto.tfvars#L79

driazati · 2022-07-25T19:51:55Z

ci/jenkins/Test.groovy.j2

    {{ method_name }}()
  },
  {% endfor %}
+  'unittest: CPU MINIMAL': {


can you use the m.test_step macro here instead? That way we don't need to duplicate all the meta-logic (timeout, workspace, etc) just for this test

I'm not sure how to do that. The test_step inlines the body while in this case, I am calling run_unittest_minimal. I could try to inline the method and use the macro, but that might not work due to the 64KB bytecode size/method limit in the JVM.

I also guess I could modify the macros.

Ok, I am adding a new macro

I meant something like https://github.com/apache/tvm/blob/main/ci/jenkins/Test.groovy.j2#L202-L218, if you do that it will automatically call the test method

Yes, but I needed a different macro that did not include the step name (eg unittest: CPU MINIMAL, frontend: aarch64 2 of 2, unittest: CPU).

https://github.com/apache/tvm/pull/12178/files#diff-1a31fe4681520821e63f0bec0a963c86b8eea7af2c7130b28eba0b987a5fc21bR86

ci/jenkins/Test.groovy.j2

tests/python/unittest/test_meta_schedule_task_scheduler.py

tests/scripts/task_config_build_minimal.sh

areusch · 2022-07-26T00:43:25Z

cc @kparzysz-quic @Mousius @manupa-arm @cbalint13 @tqchen

tests/scripts/task_config_build_minimal.sh

areusch · 2022-07-26T01:25:36Z

tests/scripts/task_cpp_unittest.sh

-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then


i really wanted to be able to have you just look this up in CMakeCache.txt, but unfortunately we don't cache these. i wonder if we should be setting this with set(USE_ABC ___ CACHED) so we can inspect that. what if we modified Summary.cmake to write the options to a file via https://cmake.org/cmake/help/latest/command/file.html#append?

it might also be cleaner to split this out into task_cpp_unittest_micro.sh or something and just add it as another step everywhere that currently runs the cpp unit tests, that way its easy to run regardless of the specific build options

I went the route of separating the script into two. Let me know your thoughts

Update: It seems that there are crt tests that get included in the cpp test suite when USE_MICRO is enabled.

Instead of separating those, I am following @areusch advice and writing to a file all the options used to build TVM.

Adds an ECR repo for `ci_minimal` Required for apache/tvm#12178

ci/jenkins/Test.groovy.j2

tests/scripts/ci.py

github-actions · 2022-07-30T00:43:39Z

Built docs for commit ea3c93e can be found here.

driazati

lgtm, waiting for approve from @areusch too before merging

tests/python/unittest/test_meta_schedule_task_scheduler.py

areusch · 2022-08-02T23:38:02Z

@gigiblender looks like just need to resolve the merge conflict

driazati · 2022-08-10T15:34:36Z

@tvm-bot merge

driazati · 2022-08-10T16:08:50Z

@tvm-bot merge

This got out of date after merging #12178 Co-authored-by: driazati <[email protected]>

* [skip ci][ci] Fix Jenkinsfile (#12387) This got out of date after merging #12178 Co-authored-by: driazati <[email protected]> * Address comments Co-authored-by: driazati <[email protected]>

This PR builds and tests TVM (running the CPP and unittests) under minimal configuration with some debug flags enabled: - `USE_RELAY_DEBUG=ON` in TVM - `-Wp,-D_GLIBCXX_ASSERTIONS` in TVM - `-DLLVM_ENABLE_ASSERTIONS=ON` in LLVM It also adds this configuration to the CI. `tests/python/unittest/test_meta_schedule_task_scheduler.py::test_meta_schedule_task_scheduler_multiple_gradient_based` results in an array OOB access and a segfault due to `D_GLIBCXX_ASSERTIONS`. I disable this test for now and will open an issue to solve it ASAP. It should fix apache#11932 and address [this discussion](https://discuss.tvm.apache.org/t/pre-rfc-new-ci-container-ci-cpu-asserts/12536/9).

This got out of date after merging apache#12178 Co-authored-by: driazati <[email protected]>

* [skip ci][ci] Fix Jenkinsfile (apache#12387) This got out of date after merging apache#12178 Co-authored-by: driazati <[email protected]> * Address comments Co-authored-by: driazati <[email protected]>

This PR builds and tests TVM (running the CPP and unittests) under minimal configuration with some debug flags enabled: - `USE_RELAY_DEBUG=ON` in TVM - `-Wp,-D_GLIBCXX_ASSERTIONS` in TVM - `-DLLVM_ENABLE_ASSERTIONS=ON` in LLVM It also adds this configuration to the CI. `tests/python/unittest/test_meta_schedule_task_scheduler.py::test_meta_schedule_task_scheduler_multiple_gradient_based` results in an array OOB access and a segfault due to `D_GLIBCXX_ASSERTIONS`. I disable this test for now and will open an issue to solve it ASAP. It should fix apache#11932 and address [this discussion](https://discuss.tvm.apache.org/t/pre-rfc-new-ci-container-ci-cpu-asserts/12536/9).

This got out of date after merging apache#12178 Co-authored-by: driazati <[email protected]>

gigiblender marked this pull request as ready for review July 25, 2022 19:28

driazati reviewed Jul 25, 2022

View reviewed changes

areusch reviewed Jul 26, 2022

View reviewed changes

gigiblender mentioned this pull request Jul 26, 2022

Add ECR repo for ci_minimal tlc-pack/ci#46

Merged

driazati pushed a commit to tlc-pack/ci that referenced this pull request Jul 26, 2022

Add ECR repo for ci_minimal (#46)

94c9206

Adds an ECR repo for `ci_minimal` Required for apache/tvm#12178

gigiblender force-pushed the tvm-minimal branch from d3cefda to ef9fc51 Compare July 27, 2022 10:22

gigiblender mentioned this pull request Jul 27, 2022

[Bug] Fix array OOB access in TVM #12199

Closed

driazati reviewed Jul 27, 2022

View reviewed changes

ci/jenkins/Test.groovy.j2 Show resolved Hide resolved

tests/scripts/ci.py Outdated Show resolved Hide resolved

driazati approved these changes Aug 1, 2022

View reviewed changes

tests/python/unittest/test_meta_schedule_task_scheduler.py Outdated Show resolved Hide resolved

gigiblender force-pushed the tvm-minimal branch from ea3c93e to b42795f Compare August 1, 2022 21:19

zxybazh reviewed Aug 2, 2022

View reviewed changes

tests/python/unittest/test_meta_schedule_task_scheduler.py Outdated Show resolved Hide resolved

gigiblender force-pushed the tvm-minimal branch 2 times, most recently from a122907 to 4911c17 Compare August 2, 2022 11:55

areusch approved these changes Aug 2, 2022

View reviewed changes

gigiblender force-pushed the tvm-minimal branch 5 times, most recently from aa0afb9 to a8f5af1 Compare August 8, 2022 19:17

Florin-Gabriel Blanaru added 3 commits August 8, 2022 13:18

Add new docker image with minimal TVM configuration

86c69fa

Add minimal TVM build and testing to Jenkinsfile

87b4005

Write TVM build options to file

7de29e5

gigiblender force-pushed the tvm-minimal branch from a8f5af1 to 6da7b61 Compare August 8, 2022 20:19

Add retry for ci_minimal

a63c4a6

gigiblender force-pushed the tvm-minimal branch from 6da7b61 to a63c4a6 Compare August 8, 2022 20:20

driazati merged commit 52152e0 into apache:main Aug 11, 2022

driazati mentioned this pull request Aug 11, 2022

[skip ci][ci] Fix Jenkinsfile #12387

Merged

areusch pushed a commit that referenced this pull request Aug 11, 2022

[skip ci][ci] Fix Jenkinsfile (#12387)

7e14441

This got out of date after merging #12178 Co-authored-by: driazati <[email protected]>

AndrewZhaoLuo mentioned this pull request Oct 4, 2022

TVM v0.10.0.rc0 Release Candidate Notes #12979

Closed

xinetzone pushed a commit to daobook/tvm that referenced this pull request Nov 25, 2022

[skip ci][ci] Fix Jenkinsfile (apache#12387)

2cf7c36

This got out of date after merging apache#12178 Co-authored-by: driazati <[email protected]>

mikeseven pushed a commit to mikeseven/tvm that referenced this pull request Sep 27, 2023

[skip ci][ci] Fix Jenkinsfile (apache#12387)

722ec4a

This got out of date after merging apache#12178 Co-authored-by: driazati <[email protected]>

Build and test TVM under minimal configuration #12178

Build and test TVM under minimal configuration #12178

Uh oh!

Conversation

gigiblender commented Jul 25, 2022 • edited by driazati Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gigiblender commented Jul 25, 2022

Uh oh!

driazati left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

areusch commented Jul 26, 2022

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gigiblender Jul 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jul 30, 2022

Uh oh!

driazati left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

areusch commented Aug 2, 2022

Uh oh!

driazati commented Aug 10, 2022

Uh oh!

driazati commented Aug 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

gigiblender commented Jul 25, 2022 •

edited by driazati

Loading

gigiblender Jul 27, 2022 •

edited

Loading