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/12/10 06:44:51 UTC

[GitHub] [tvm] wrongtest-intellif opened a new pull request, #13591: [LLVM] Fix get tm allow_missing check pos

wrongtest-intellif opened a new pull request, #13591:
URL: https://github.com/apache/tvm/pull/13591

   It seems that `GetOrCreateTargetMachine`'s failure check would be out of `lookupTarget`' nest scope.


-- 
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] wrongtest-intellif commented on a diff in pull request #13591: [LLVM] Fix get tm allow_missing check pos

Posted by GitBox <gi...@apache.org>.
wrongtest-intellif commented on code in PR #13591:
URL: https://github.com/apache/tvm/pull/13591#discussion_r1046620515


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -297,9 +297,9 @@ llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing
         llvm_instance->createTargetMachine(triple_, cpu_, GetTargetFeatureString(), target_options_,
                                            reloc_model_, code_model_, opt_level_);
     target_machine_ = std::unique_ptr<llvm::TargetMachine>(tm);
-    if (!allow_missing) {

Review Comment:
   Emmmm but when `llvm::TargetRegistry::lookupTarget(triple_, error)` failed,the `target_machine_`  is also nullptr,



-- 
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] tvm-bot commented on pull request #13591: [LLVM] Fix get tm allow_missing check pos

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

   <!---bot-comment-->
   
   Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @-ing them in a comment.
   
   <!--bot-comment-ccs-start-->
    * No users to tag found in teams: `llvm` <sub>See [#10317](https://github.com/apache/tvm/issues/10317) for details</sub><!--bot-comment-ccs-end-->
   
   <sub>Generated by [tvm-bot](https://github.com/apache/tvm/blob/main/ci/README.md#github-actions)</sub>


-- 
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] echuraev merged pull request #13591: [LLVM] Fix get tm allow_missing check pos

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


-- 
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 a diff in pull request #13591: [LLVM] Fix get tm allow_missing check pos

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #13591:
URL: https://github.com/apache/tvm/pull/13591#discussion_r1047354819


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -297,9 +297,9 @@ llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing
         llvm_instance->createTargetMachine(triple_, cpu_, GetTargetFeatureString(), target_options_,
                                            reloc_model_, code_model_, opt_level_);
     target_machine_ = std::unique_ptr<llvm::TargetMachine>(tm);
-    if (!allow_missing) {

Review Comment:
   Ah, you're right.



-- 
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 a diff in pull request #13591: [LLVM] Fix get tm allow_missing check pos

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #13591:
URL: https://github.com/apache/tvm/pull/13591#discussion_r1045925324


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -297,9 +297,9 @@ llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing
         llvm_instance->createTargetMachine(triple_, cpu_, GetTargetFeatureString(), target_options_,
                                            reloc_model_, code_model_, opt_level_);
     target_machine_ = std::unique_ptr<llvm::TargetMachine>(tm);
-    if (!allow_missing) {

Review Comment:
   This is the only place where `target_machine_` is assigned a value, so this check was actually sufficient.



-- 
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] wrongtest-intellif commented on pull request #13591: [LLVM] Fix get tm allow_missing check pos

Posted by GitBox <gi...@apache.org>.
wrongtest-intellif commented on PR #13591:
URL: https://github.com/apache/tvm/pull/13591#issuecomment-1345829858

   cc @kparzysz-quic 


-- 
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 a diff in pull request #13591: [LLVM] Fix get tm allow_missing check pos

Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #13591:
URL: https://github.com/apache/tvm/pull/13591#discussion_r1045925324


##########
src/target/llvm/llvm_instance.cc:
##########
@@ -297,9 +297,9 @@ llvm::TargetMachine* LLVMTargetInfo::GetOrCreateTargetMachine(bool allow_missing
         llvm_instance->createTargetMachine(triple_, cpu_, GetTargetFeatureString(), target_options_,
                                            reloc_model_, code_model_, opt_level_);
     target_machine_ = std::unique_ptr<llvm::TargetMachine>(tm);
-    if (!allow_missing) {

Review Comment:
   This is the only place where `target_machine_` is assigned a value, so this check was actually sufficient.
   
   Edit: the change doesn't break anything, so it can stay.



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