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 2020/09/30 21:31:40 UTC
[GitHub] [incubator-tvm] iiahim opened a new pull request #6600: Updated runtime to run under FreeBSD.
iiahim opened a new pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600
setenv CXX to proper binary - c++ or g++9 for FreeBSD 12.0.
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#issuecomment-702272065
Let's fix the linter issue and get it merged. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498396804
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
Sure. I don't know how to update the line to 'os.environ.get("CXX", "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None)' - I can not edit the suggestion.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498446609
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
I hope I checked in the fix in proper place.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498376103
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
What is the flow to get this update?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r497824388
##########
File path: python/tvm/rpc/server.py
##########
@@ -73,14 +73,17 @@ def load_module(file_name):
@tvm._ffi.register_func("tvm.rpc.server.download_linked_module", override=True)
def download_linked_module(file_name):
"""Load module from remote side."""
+ # c++ compiler/linker
+ cc = os.environ['CXX'] if 'CXX' in os.environ.keys() else "g++"
Review comment:
```suggestion
cc = os.environ.get("CXX", "g++")
```
##########
File path: python/tvm/runtime/module.py
##########
@@ -393,13 +394,17 @@ def load_module(path, fmt=""):
This function will automatically call
cc.create_shared if the path is in format .o or .tar
"""
+
+ # c++ compiler/linker
+ cc = os.environ["CXX"] if "CXX" in os.environ.keys() else "g++"
Review comment:
```suggestion
cc = os.environ.get("CXX", "g++")
```
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
```suggestion
os.environ.get("CXX", "g++") if sys.platform == "darwin" or sys.platform.startswith("linux") else None
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r497849264
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
I don't think this one works. The original function is the CXX env will be used even if the system is not linux or darwin.
The suggested change does not do that, will set the default value only for linux or darwin, which is not the intent.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#issuecomment-702738795
Thanks @iiahim @junrushao1994 !
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r497865792
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
May be 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498391587
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
Typically we just use g++. On macos, g++ is symlinked to clang++, so it is fine imo
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#issuecomment-702414115
Looks like:
1. docker/lint.sh pylint
2. docker/bash.sh tlcpack/ci-lint:v0.62 ./tests/scripts/task_lint.sh
Produce different results - I've run initially 1) and was clean, 2) seems more picky.
I've got 1) from somewhere on the doc ... may be needs an update[?].
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r497865717
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
`os.environ.get("CXX", "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None)`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498377823
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
Also ... is any reason to use g++ and not c++?
While I thing getting it from the env if it set has values c++ seems to work on Linux (Ubuntu & Centos) and FreeBSD. I don't have access to MacOS to test it there.
Would you accept to change g++ -> c++ as default?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen commented on pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#issuecomment-702415351
do `./tests/lint/git-black -i upstream/master`
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#issuecomment-702417441
I've looked at scripts ... probably the one failing was python_format. It is not in the list of tasks in ./docker/lint.sh - if it was I would have run it. May be it's a good idea to be added since doc writes about running ./docker/lint.sh.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] iiahim commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
iiahim commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r498377823
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
Also ... is any reason to use g++ and not c++?
While I think getting it from the env if it has values, c++ seems to work on Linux (Ubuntu & Centos) and FreeBSD. I don't have access to MacOS to test it there.
Would you accept to change g++ -> c++ as default?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] junrushao1994 commented on a change in pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600#discussion_r497885939
##########
File path: python/tvm/contrib/cc.py
##########
@@ -103,7 +105,8 @@ def get_target_triple():
# assign so as default output format
create_shared.output_format = "so" if sys.platform != "win32" else "dll"
create_shared.get_target_triple = get_target_by_dump_machine(
- "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
+ os.environ["CXX"] if "CXX" in os.environ.keys() else \
+ "g++" if sys.platform == "darwin" or sys.platform.startswith("linux") else None
Review comment:
Yeah this one is better. 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-tvm] tqchen merged pull request #6600: Updated runtime to run under FreeBSD.
Posted by GitBox <gi...@apache.org>.
tqchen merged pull request #6600:
URL: https://github.com/apache/incubator-tvm/pull/6600
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org