[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-572418800 Now, we could have new elegant and simpler automatic detection of LLVM target triple (thanks @tqchen 's good idea). @tqchen @zhiics could you help to review new code again? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-572375077 > > if cl is used, I think we can safely assume the target is windows, we only need to know whether if it is win32 or win64, i am not that familar with cl to know for sure, but i guess in that case we can pass and use LLVM module detection, or fallback to c > > Yes. We could assume the target is Windows for sure. However, i think besides we need to know win32 / win64, one more thing I am a little worried. Windows could run ARM CPU (`cl.exe` could run Windows 10 on ARM? I can not make sure yet too). Ah...One tricky way I just see on the YoutuBe...  Maybe we could use `DUMPBIN` of `cl.exe` to make sure it is ARM or x86...But, it is so tricky, I personally think we maybe could fallback to c on Windows. Because I think ``` If the module lists already contains an LLVM module, we can get triples from those modules ``` could solve most of our cases. How about your opinion? 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-572373834 > if cl is used, I think we can safely assume the target is windows, we only need to know whether if it is win32 or win64, i am not that familar with cl to know for sure, but i guess in that case we can pass and use LLVM module detection, or fallback to c Yes. We could assume the target is Windows for sure. However, i think besides we need to know win32 / win64, one more thing I am a little worried. Windows could run ARM CPU (`cl.exe` could run Windows 10 on ARM? I can not make sure yet too). 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-572361186 > Some ideas about automatic detection, in the order of things that can be tried > > * If the module lists already contains an LLVM module, we can get triples from those modules > * The property could be part of fcompile.get_target_triple() > > * Use hasattr to detect if fcompile contains the property > * The function can return None, which means unable to detect > * Note that for gcc and clang `gcc -dumpmachine` will give you the triple > * Add support for this function in create_shared(using `gcc -dumpmachine`) > * If we cannot detect using the above approach, fallback to PackToC, note that this is super unlikely Good idea. One quick question, how about Windows's compiler `cl`? The `cl` (https://docs.microsoft.com/en-us/cpp/build/reference/compiler-options-listed-by-category?view=vs-2019) seems doesn't have option to show arch information of its self. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-57539
> If we indeed needs to pass target triple, perhaps the best way to do so is
to pass that as an argument of PackImportToLLVM
So, your opinion is let the users specify. The API will become this:
```
def export_library(self,
file_name,
fcompile=None,
llvm_target=None,
**kwargs):
if enabled("llvm"):
path_obj = temp.relpath("devc.o")
m = _PackImportsToLLVM(self, is_system_lib, llvm_target)
```
If users doesn't set `llvm_target`, we use the default value. (system target
triple).
if users want to compile into another os / another cpu, they have to do this:
```module.export_library(..., llvm_target="llvm -target
aarch64-linux-gnu-none")``` now.
Does it make sense to you?
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly
FrozenGene commented on issue #4657: [CodeGen] Generate blob use LLVM directly URL: https://github.com/apache/incubator-tvm/pull/4657#issuecomment-57213 @tqchen @zhiics About the LLVM target part, I also think current way maybe is not good. I also think of it and cost my some time. So I want to discuss it too. Firstly Let me describe why we need llvm target host. When we use PackImportToLLVM, we will need to use LLVM to create LLVM module. However, we must need one target machine, this is answered to @tqchen 's question. We can not create one generic target. Because we have platform's specific feature to handle. See:https://github.com/apache/incubator-tvm/pull/4657/files#diff-f3a67dc7be877c1da11892f6a8e5ae80R107-R109 and https://github.com/apache/incubator-tvm/pull/4657/files#diff-f3a67dc7be877c1da11892f6a8e5ae80R107-R109. Another thing is if we leave the target is empty, i.e. no module->setTargetTriple(target_triple.str()); On llvm 6.0 of Mac will report problem : assert error Target-incompatible DataLayout. More import thing is we need to consider the target host is not x86 cpu. For example, target host is llvm -target aarch64, if we leave it empty and build it on x86 server, we will generate devc.o into x86 format, but in fact we want aarch64 of devc.o. So in the codegen_blob part, we should know the correct llvm target host to generate correct file. Current way is a little ugly I think too. Current way will create target host based on LLVM when we have detected target host finally (because tvm.build / relay.build's api could leave `target_host` be None, we have some logic to handle this situation). One simple way of course is let users specific the target host of llvm representation, but I use this ugly way is just to detect it automatically and don't want to let users do. I also want to listen to your advices. Thanks in advance. 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: us...@infra.apache.org With regards, Apache Git Services
