You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/07/25 18:21:55 UTC

[GitHub] [tvm] gigiblender opened a new pull request, #12178: Build and test TVM under minimal configuration

gigiblender opened a new pull request, #12178:
URL: https://github.com/apache/tvm/pull/12178

   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.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r931546473


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +217,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {
+    run_unittest_minimal()
+  },

Review Comment:
   Not really. The `test_step` macro will expand and generate something as if the body of run_unittest_minimal was inlined at the call site. 
   
   Due to the JVM method size limit, I cannot inline the body of `run_unittest_minimal` inside the Jenkinsfile. 
   
   Therefore, I need a macro that does the same thing as `test_step` but does not include the declaration that is part of the `parallel`:  `unittest: CPU MINIMAL: {`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r934818576


##########
tests/python/unittest/test_meta_schedule_task_scheduler.py:
##########
@@ -337,6 +338,18 @@ def test_meta_schedule_task_scheduler_override_next_task_id_only():  # pylint: d
         )
 
 
+def should_skip_oob_test():
+    flags = libinfo()
+    # TODO(gigiblender) This combination of flags should be enabled only in the ci_minimal CI configuration.
+    #  Remove this once the OOB test is fixed.
+    return (
+        flags["USE_RELAY_DEBUG"] == "ON"
+        and flags["USE_LLVM"] == "llvm-config"
+        and flags["USE_MICRO"] != "ON"
+    )
+
+
+@pytest.mark.skipif(should_skip_oob_test(), reason="Does array access OOB. Remove once fixed.")

Review Comment:
   There's this issue open: https://github.com/apache/tvm/issues/12199
   
   Do you mean to add it in a comment?
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1210879967

   @tvm-bot merge


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati merged pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati merged PR #12178:
URL: https://github.com/apache/tvm/pull/12178


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r934769755


##########
tests/python/unittest/test_meta_schedule_task_scheduler.py:
##########
@@ -337,6 +338,18 @@ def test_meta_schedule_task_scheduler_override_next_task_id_only():  # pylint: d
         )
 
 
+def should_skip_oob_test():
+    flags = libinfo()
+    # TODO(gigiblender) This combination of flags should be enabled only in the ci_minimal CI configuration.
+    #  Remove this once the OOB test is fixed.
+    return (
+        flags["USE_RELAY_DEBUG"] == "ON"
+        and flags["USE_LLVM"] == "llvm-config"
+        and flags["USE_MICRO"] != "ON"
+    )
+
+
+@pytest.mark.skipif(should_skip_oob_test(), reason="Does array access OOB. Remove once fixed.")

Review Comment:
   still needs a GitHub issue + link here



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930913205


##########
tests/scripts/task_cpp_unittest.sh:
##########
@@ -45,20 +45,23 @@ python3 tests/scripts/task_build.py \
     --cmake-target cpptest \
     --build-dir "${BUILD_DIR}"
 
-# crttest requires USE_MICRO to be enabled, which is currently the case
-# with all CI configs
-pushd "${BUILD_DIR}"
-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then

Review Comment:
   I went the route of separating the script into two



##########
tests/scripts/task_cpp_unittest.sh:
##########
@@ -45,20 +45,23 @@ python3 tests/scripts/task_build.py \
     --cmake-target cpptest \
     --build-dir "${BUILD_DIR}"
 
-# crttest requires USE_MICRO to be enabled, which is currently the case
-# with all CI configs
-pushd "${BUILD_DIR}"
-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then

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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r929445420


##########
tests/scripts/task_cpp_unittest.sh:
##########
@@ -45,20 +45,23 @@ python3 tests/scripts/task_build.py \
     --cmake-target cpptest \
     --build-dir "${BUILD_DIR}"
 
-# crttest requires USE_MICRO to be enabled, which is currently the case
-# with all CI configs
-pushd "${BUILD_DIR}"
-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then

Review Comment:
   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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r931425116


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +217,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {
+    run_unittest_minimal()
+  },

Review Comment:
   I still don't understand why we need a new macro, does this change not do the same thing with existing code?
   ```suggestion
     {% call m.test_step(
       name="unittest: CPU MINIMAL",
       node="CPU-SMALL",
       ws="tvm/ut-python-cpu-minimal",
       platform="minimal",
       docker_image="ci_minimal",
     ) %}
       {{ m.download_artifacts(tag='cpu-minimal', filenames=tvm_lib) }}
       cpp_unittest(ci_minimal)
       python_unittest(ci_minimal)
     {% endcall %}
   ```



##########
tests/scripts/ci.py:
##########
@@ -595,11 +601,25 @@ def add_subparser(
             "frontend": ("run frontend tests", ["./tests/scripts/task_python_frontend_cpu.sh"]),
         },
     ),
+    generate_command(
+        name="minimal",
+        help="Run minimal CPU build and test(s)",
+        options={
+            "cpp": CPP_UNITTEST,
+            "unittest": (
+                "run unit tests",
+                [
+                    "./tests/scripts/task_python_unittest.sh",
+                ],
+            ),
+        },
+    ),
     generate_command(
         name="i386",
         help="Run i386 build and test(s)",
         options={
             "cpp": CPP_UNITTEST,
+            "micro_cpp": CPP_MICRO_UNITTEST,

Review Comment:
   iirc these turn into command line flags
   
   ```suggestion
               "micro-cpp": CPP_MICRO_UNITTEST,
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r929248440


##########
tests/scripts/task_config_build_minimal.sh:
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -euxo pipefail
+
+BUILD_DIR=$1
+mkdir -p "$BUILD_DIR"
+cd "$BUILD_DIR"
+cp ../cmake/config.cmake .
+
+echo set\(USE_SORT ON\) >> config.cmake

Review Comment:
   We've had some issues come up before where building in debug mode surfaced errors that weren't there in release mode, this is somewhat of a different discussion but I wonder if we should also set `CMAKE_BUILD_TYPE=Debug` here to check that in CI (I guess the question is whether that should be its own test or rolled into this one)



##########
ci/jenkins/Test.groovy.j2:
##########
@@ -187,6 +187,41 @@
   )
 {% endcall %}
 
+def run_unittest_minimal() {
+  if (!skip_ci && is_docs_only_build != 1) {
+    node('CPU-SMALL') {
+      ws({{ m.per_exec_ws('tvm/ut-python-cpu-minimal') }}) {
+        timeout(time: max_time, unit: 'MINUTES') {
+          try {
+            docker_init(ci_minimal)
+            init_git()
+            withEnv(['PLATFORM=minimal'], {
+              sh(

Review Comment:
   can you also use the `m.download_artifacts` macro here?



##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +234,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {

Review Comment:
   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



##########
tests/python/unittest/test_meta_schedule_task_scheduler.py:
##########
@@ -337,6 +337,7 @@ def test_meta_schedule_task_scheduler_override_next_task_id_only():  # pylint: d
         )
 
 
+@pytest.mark.skip("Does array access OOB")

Review Comment:
   Can you file and link an issue here with the context? Also it'd be nice if this was a `skipIf`, maybe you can use a combo of flags from libinfo https://github.com/apache/tvm/blob/main/src/support/libinfo.cc#L238?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r929444267


##########
tests/scripts/task_cpp_unittest.sh:
##########
@@ -45,20 +45,23 @@ python3 tests/scripts/task_build.py \
     --cmake-target cpptest \
     --build-dir "${BUILD_DIR}"
 
-# crttest requires USE_MICRO to be enabled, which is currently the case
-# with all CI configs
-pushd "${BUILD_DIR}"
-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then

Review Comment:
   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](https://github.com/apache/tvm/blob/main/cmake/utils/Summary.cmake) to write the options to a file via https://cmake.org/cmake/help/latest/command/file.html#append?



##########
tests/scripts/task_config_build_minimal.sh:
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -euxo pipefail
+
+BUILD_DIR=$1
+mkdir -p "$BUILD_DIR"
+cd "$BUILD_DIR"
+cp ../cmake/config.cmake .
+
+echo set\(USE_SORT ON\) >> config.cmake

Review Comment:
   +1 to this



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1203321700

   @gigiblender looks like just need to resolve the merge conflict


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930897628


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +234,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {

Review Comment:
   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



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930059275


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +234,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {

Review Comment:
   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. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1210931462

   @tvm-bot merge


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930912630


##########
tests/scripts/task_config_build_minimal.sh:
##########
@@ -0,0 +1,33 @@
+#!/usr/bin/env bash
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+set -euxo pipefail
+
+BUILD_DIR=$1
+mkdir -p "$BUILD_DIR"
+cd "$BUILD_DIR"
+cp ../cmake/config.cmake .
+
+echo set\(USE_SORT ON\) >> config.cmake

Review Comment:
   Enabled debug mode. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] areusch commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
areusch commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1194829695

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r931570525


##########
tests/scripts/ci.py:
##########
@@ -595,11 +601,25 @@ def add_subparser(
             "frontend": ("run frontend tests", ["./tests/scripts/task_python_frontend_cpu.sh"]),
         },
     ),
+    generate_command(
+        name="minimal",
+        help="Run minimal CPU build and test(s)",
+        options={
+            "cpp": CPP_UNITTEST,
+            "unittest": (
+                "run unit tests",
+                [
+                    "./tests/scripts/task_python_unittest.sh",
+                ],
+            ),
+        },
+    ),
     generate_command(
         name="i386",
         help="Run i386 build and test(s)",
         options={
             "cpp": CPP_UNITTEST,
+            "micro_cpp": CPP_MICRO_UNITTEST,

Review Comment:
   The `_` are replaced with `-` by the `cli_name` function, so the command line flag available and shown in `--help` is `micro-cpp`.
   
   Strangely, if I replace `micro_cpp` with `micro-cpp` then calling, for example, `python tests/scripts/ci.py cpu --micro-cpp` will only run the build. This might be a limitation in how we generate the argument parsers.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] github-actions[bot] commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1200051456

   <!---docs-bot-comment-->
   
   Built docs for commit ea3c93e5c890a4545e1648c9a0f9e47936324ade can be found [here](https://pr-docs.tlcpack.ai/PR-12178/5/docs/index.html).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r934819147


##########
tests/python/unittest/test_meta_schedule_task_scheduler.py:
##########
@@ -337,6 +338,18 @@ def test_meta_schedule_task_scheduler_override_next_task_id_only():  # pylint: d
         )
 
 
+def should_skip_oob_test():
+    flags = libinfo()
+    # TODO(gigiblender) This combination of flags should be enabled only in the ci_minimal CI configuration.
+    #  Remove this once the OOB test is fixed.
+    return (
+        flags["USE_RELAY_DEBUG"] == "ON"
+        and flags["USE_LLVM"] == "llvm-config"
+        and flags["USE_MICRO"] != "ON"
+    )
+
+
+@pytest.mark.skipif(should_skip_oob_test(), reason="Does array access OOB. Remove once fixed.")

Review Comment:
   Yeah it should be mentioned in the `reason` so it shows up in the pytest results



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930113155


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +234,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {

Review Comment:
   Ok, I am adding a new macro



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] driazati commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
driazati commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r930197046


##########
ci/jenkins/Test.groovy.j2:
##########
@@ -199,6 +234,9 @@ stage('Test') {
     {{ method_name }}()
   },
   {% endfor %}
+  'unittest: CPU MINIMAL': {

Review Comment:
   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 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on PR #12178:
URL: https://github.com/apache/tvm/pull/12178#issuecomment-1194522328

   @areusch @driazati 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] zxybazh commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
zxybazh commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r935114028


##########
tests/python/unittest/test_meta_schedule_task_scheduler.py:
##########
@@ -337,6 +338,23 @@ def test_meta_schedule_task_scheduler_override_next_task_id_only():  # pylint: d
         )
 
 
+def should_skip_oob_test():
+    flags = libinfo()
+    # TODO(gigiblender) This combination of flags should be enabled only in the ci_minimal CI configuration.
+    #  Remove this once the OOB test is fixed.
+    return (
+        flags["USE_RELAY_DEBUG"] == "ON"
+        and flags["USE_LLVM"] == "llvm-config"
+        and flags["USE_MICRO"] != "ON"
+    )
+
+
+@pytest.mark.skipif(

Review Comment:
   Hi, can you please enable this test after rebase to latest main because it’s been fixed, thanks!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [tvm] gigiblender commented on a diff in pull request #12178: Build and test TVM under minimal configuration

Posted by GitBox <gi...@apache.org>.
gigiblender commented on code in PR #12178:
URL: https://github.com/apache/tvm/pull/12178#discussion_r933383395


##########
tests/scripts/task_cpp_unittest.sh:
##########
@@ -45,20 +45,23 @@ python3 tests/scripts/task_build.py \
     --cmake-target cpptest \
     --build-dir "${BUILD_DIR}"
 
-# crttest requires USE_MICRO to be enabled, which is currently the case
-# with all CI configs
-pushd "${BUILD_DIR}"
-ninja crttest
-popd
+# crttest requires USE_MICRO to be enabled.
+if grep -Fq "set(USE_MICRO ON)" ${BUILD_DIR}/config.cmake; then

Review Comment:
   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.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org