[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4657: [CodeGen] Generate blob use LLVM directly

2020-01-08 Thread GitBox
FrozenGene edited a comment 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...
   
![image](https://user-images.githubusercontent.com/7287321/72037208-42026780-32d8-11ea-902a-5048249e4cfa.png)
   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 if we have fallen into `fcompile.get_target_triple()`. 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? @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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services


[GitHub] [incubator-tvm] FrozenGene edited a comment on issue #4657: [CodeGen] Generate blob use LLVM directly

2020-01-08 Thread GitBox
FrozenGene edited a comment 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...
   
![image](https://user-images.githubusercontent.com/7287321/72037208-42026780-32d8-11ea-902a-5048249e4cfa.png)
   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 if we have fallen into `fcompile.get_target_triple()`. 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 edited a comment on issue #4657: [CodeGen] Generate blob use LLVM directly

2020-01-08 Thread GitBox
FrozenGene edited a comment 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
   
   @tqchen  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 edited a comment on issue #4657: [CodeGen] Generate blob use LLVM directly

2020-01-08 Thread GitBox
FrozenGene edited a comment 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/blob/4f1772c0598ae708477f457a993f5be52fa71eb9/src/codegen/llvm/codegen_blob.cc#L107
 and 
https://github.com/apache/incubator-tvm/blob/4f1772c0598ae708477f457a993f5be52fa71eb9/src/codegen/llvm/codegen_blob.cc#L167.
 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