-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix -mfloat-abi=soft compilation for ARM with OpenCL target #6150
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks Trevor!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
cf1db08
to
19a7795
Compare
Looks like some unrelated failures in ci?
|
Yes, looks like the CI failure is not related |
19a7795
to
01d6a32
Compare
01d6a32
to
647b7b9
Compare
@kevinthesun CI is passing now |
Currently, any extra attributes of the llvm target are not being passed through to LLVMModule during
ModulePackImportsToLLVM
. This will cause compilation to fail witherror: /tmp/tmpctakbpk1/devc.o uses VFP register arguments, output does not
when the target is opencl and the target_host requires-mfloat-abi=soft
. This regression was introduced by #4657.There are two reasons why:
python/tvm/runtime/module.py
uses "_get_target_triple" to get the target triple to use forModulePackImportsToLLVM
. However, the implementation of that function returnstm_->getTargetTriple().str()
which will only have the standard<arch><sub>-<vendor>-<sys>-<abi>
and is missing any extra flags or attributes such as-mfloat-abi=soft
.module_->getTargetTriple()
is called which again doesn't have the extra flags and attributes: https://github.com/apache/incubator-tvm/blob/master/src/target/llvm/llvm_module.cc#L254This PR fixes those problems by:
_get_target_triple
(only mfloat-abi for now). This part doesn't seem ideal to me. Any thoughts on how to improve? Can we store the user's full target string earlier on?CodeGenBlob
so thatLLVMModuleNode::Init(std::unique_ptr<llvm::Module> module, std::shared_ptr<llvm::LLVMContext> ctx)
can retrieve it: https://github.com/apache/incubator-tvm/blob/master/src/target/llvm/llvm_module.cc#L247 This matches whatLLVMModuleNode::Init(const IRModule& mod, std::string target)
already does.Discuss post: https://discuss.tvm.ai/t/opencl-target-for-32-bit-arm-linux-android-broken-after-pr-4657/7252
Fixes #6019