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/06/23 15:01:42 UTC

[GitHub] [tvm] kparzysz-quic opened a new pull request, #11852: [LLVM] Register factory function for CodeGenCPU

kparzysz-quic opened a new pull request, #11852:
URL: https://github.com/apache/tvm/pull/11852

   Any target that has its own subclass of `CodeGenLLVM` must register a factory function that constructs an object of that class. This factory will then be looked up and used in `CodeGenLLVM::Create`, which is the generic interface to create an LLVM code generator.
   
   However, there is no factory for `CodeGenCPU`, and so the creation of a `CodeGenCPU` object is done inside of `CodeGenLLVM::Create`. To make this happen, codegen_llvm.cc includes codegen_cpu.h, which makes the base class implementation depend on the derived class. This backwards dependency can be resolved by registering a factory for `CodeGenCPU`.
   


-- 
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] kparzysz-quic commented on pull request #11852: [LLVM] Register factory function for CodeGenCPU

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #11852:
URL: https://github.com/apache/tvm/pull/11852#issuecomment-1164782629

   I'll paste the commit comment about why I deleted the cpp test:
   
   The WASM docker has an installation of LLVM without versioned symbols, but the default compiler uses symbol versions. The added test had a reference to `_ZN4llvm24DisableABIBreakingChecksE@@LLVM_11`, but the LLVM library had the unversioned symbol:
   ```
   File: /usr/lib/llvm-11/lib/libLLVMSupport.a(ABIBreak.cpp.o)
        1: 0000000000000000     4 OBJECT  GLOBAL DEFAULT    2 _ZN4llvm24DisableABIBreakingChecksE
   ```


-- 
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] leandron commented on pull request #11852: [LLVM] Register factory function for CodeGenCPU

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

   Thanks for the PR @kparzysz-quic. Is there any chance we can have a test case for this fix?


-- 
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] leandron merged pull request #11852: [LLVM] Register factory function for CodeGenCPU

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


-- 
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] leandron commented on pull request #11852: [LLVM] Register factory function for CodeGenCPU

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

   > I'm not sure what such a testcase should do. The main motivation for this change was to avoid the "reverse dependency" between .cc and .h files. The aren't any tests that I know of that are specific to the factory functions, but if you have any suggestions, let me know.
   
   Following the logic being introduced in this PR, I suppose tests could assert that two calls for the factory function will generate two different instances; another test could assert that an invalid target would return `nullptr`.
   
   Does that make sense?


-- 
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] kparzysz-quic commented on pull request #11852: [LLVM] Register factory function for CodeGenCPU

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #11852:
URL: https://github.com/apache/tvm/pull/11852#issuecomment-1164590030

   I'm not sure what such a testcase should do.  The main motivation for this change was to avoid the "reverse dependency" between .cc and .h files.  The aren't any tests that I know of that are specific to the factory functions, but if you have any suggestions, let me know.


-- 
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