You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by "cbalint13 (via GitHub)" <gi...@apache.org> on 2024/03/28 00:18:24 UTC
[PR] Fix errors during loading of target tags [tvm]
cbalint13 opened a new pull request, #16808:
URL: https://github.com/apache/tvm/pull/16808
This PR fix tvm loading errors when lower LLVM version does not support TVM predefined tags.
Report and related discussions are here: https://github.com/apache/tvm/pull/16425#issuecomment-2023292845
---
#### Rationale:
* During tvm load/initialization [list_tags()](https://github.com/apache/tvm/blob/83e7e9b2eb8dbeeb16dcfdbaf3336caa81071877/python/tvm/target/tag.py#L71) checks all ```src/target/tag.cc``` target definitions
* The tags **should be protected** against improper LLVM versions at their declaration time
#### Other fixes:
* Fixes the ```mcpu=generic``` fallback assignment, so LLVM will not be invoked improperly (no spurr LLVM noise)
* Minor compilation fix for LLVM version ranges: 10 < llvm < 16 (introduced by SVE support)
Cc @Lunderberg @lhutton1
--
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
Re: [PR] Fix errors during loading of target tags [tvm]
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542623104
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
Curious about this change, I tried building with llvm-13 locally but it failed. Also it seems the build for ci-cpu (which uses llvm-15) also failed: https://ci.tlcpack.ai/blue/organizations/jenkins/tvm-cpu/detail/PR-16808/2/pipeline. What version of llvm caused the failure?
--
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
Re: [PR] Fix errors during loading of target tags [tvm]
Posted by "lhutton1 (via GitHub)" <gi...@apache.org>.
lhutton1 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542645230
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
Thanks, that works now :)
--
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
Re: [PR] Fix errors during loading of target tags [tvm]
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542638093
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
@lhutton1
* Updated now.
```
+#if TVM_LLVM_VERSION >= 120
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
```
I double checked compilation with llvm={10,11,12,13,14,15,16,17,18} .
Later EDIT:
* only ```codegen_llvm.cc``` compilation is tested through.
* in full (whole TVM) is tested with llvm={10,11,16,17,18}
--
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
Re: [PR] Fix errors during loading of target tags [tvm]
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542665071
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
@lhutton1
> Thanks, that works now :)
Also thanks for the catch !
Sorry for the confusion my double checks took **some longer time**
The threshold of ```getter``` turned to be at ```>= 120``` after exhaustive checks.
--
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
Re: [PR] [Runtime][LLVM] Fix errors during loading of target tags [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg merged PR #16808:
URL: https://github.com/apache/tvm/pull/16808
--
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
Re: [PR] Fix errors during loading of target tags [tvm]
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542638093
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
@lhutton1
* Updated now.
```
+#if TVM_LLVM_VERSION >= 120
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
```
I double checked compilation with llvm={10,11,12,13,14,15,16,17,18} .
--
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
Re: [PR] [Runtime][LLVM] Fix errors during loading of target tags [tvm]
Posted by "cbalint13 (via GitHub)" <gi...@apache.org>.
cbalint13 commented on code in PR #16808:
URL: https://github.com/apache/tvm/pull/16808#discussion_r1542638093
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -1871,7 +1871,11 @@ llvm::Value* CodeGenLLVM::VisitExpr_(const BroadcastNode* op) {
value = builder_->CreateInsertElement(undef, value, zero);
#if TVM_LLVM_VERSION >= 110
llvm::ElementCount ec =
+#if TVM_LLVM_VERSION >= 160
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
Review Comment:
@lhutton1
* Updated now.
```
+#if TVM_LLVM_VERSION >= 120
llvm::ElementCount::get(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#else
+ llvm::ElementCount(dtype.get_lanes_or_vscale_factor(), dtype.is_scalable_vector());
+#endif
```
I double checked compilation with llvm={10,11,12,13,14,15,16,17,18} .
Later EDIT:
* only ```codegen_llvm.cc``` compilation is tested through.
* in full (whole TVM) is tested with llvm={10,11,16,17}
--
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
Re: [PR] [Runtime][LLVM] Fix errors during loading of target tags [tvm]
Posted by "Lunderberg (via GitHub)" <gi...@apache.org>.
Lunderberg commented on PR #16808:
URL: https://github.com/apache/tvm/pull/16808#issuecomment-2027196038
Thank you for the fix, and I can confirm that it avoids having the error-on-import. I took a brief look as well, but couldn't find an easy way to query whether a given `mcpu` argument would be valid for a known target-triple without triggering the internal LLVM error message.
--
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