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/07/19 22:05:44 UTC
[GitHub] [tvm] kparzysz-quic opened a new pull request, #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
kparzysz-quic opened a new pull request, #12140:
URL: https://github.com/apache/tvm/pull/12140
This implements RFC 80. See https://github.com/apache/tvm-rfcs/pull/83.
Summary of changes:
- Created an `LLVMScope` class. Uses of LLVM functions and data structures should be contained within the lifetime of an object of this class. LLVMScope object contains LLVMContext, and implements member functions to deserialize an llvm::Module.
- Created an `LLVMTarget` class. Once an LLVMScope object has been created, an object of LLVMTarget class can be created from TVM target string, or Target object for "llvm" target. Once LLVM command line flags are added to the "llvm" target, one of the goals of this object will be to save/restore relevant LLVM global state. Another objective for the LLVMTarget object is to be a single location for all LLVM-related compilation structures and options (such as TargetMachine, FastMathFlags, etc.)
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204077887
Originally the scope took effect immediately, but there was an ordering problem with deserializing LLVM IR: LLVM flags would only be known once we've made LLVM calls, and once we've created an LLVM context. So later in the RFC discussion I proposed separating the scope into two parts: one part that encloses the LLVM context creation, and one that saves/restores LLVM options.
I'd be happy to make further improvements to this, as we decide going on in the future.
--
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] tqchen commented on pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
tqchen commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204060680
Sorry for delayed reply. Approving as it is a good chunk of improvement. It would be great to discuss naming and documentation a bit.
When I see LLVMScope, I was expecting LLVM to taken in-effect. But in reality, we can have multiple `LLVMScope` object, one per LLVMModule. The API kicks in when we do
```c++
With<LLVMTarget> target(scope, ...)
```
So LLVMScope is not exactly an unique RAII object. Sorry that I missed it when reading the RFC but it becomes obvious when reading implementation of LLVMModule.
Perhaps we should consider clarify and rename in this case.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204259018
What do you think about `LLVMInstance`?
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1201673186
Is it ok to merge?
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1203222578
@tqchen: are you ok with this?
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r928888179
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
+static const llvm::CodeGenOpt::Level opt_level = llvm::CodeGenOpt::Aggressive;
+} // namespace defaults
+} // namespace
+
+namespace {
+bool InitializeLLVM() {
+ static std::atomic_flag initialized = ATOMIC_FLAG_INIT;
Review Comment:
The previous initialization code (in [llvm_common.cc](https://github.com/apache/tvm/pull/12140/files#diff-87ff91bdb4a2845d4ffac8e46ea9648cb72ae71505530a420163202c8906d373L64)) had an atomic flag in it (although in a bit more complex scheme), so I kept it. If you still think it should be removed, let me know---I have no problem changing it.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r928892200
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
Review Comment:
Because the cpplinter doesn't like global `std::string`s. Since strings are represented by `std::string` in the code, I kept this variable as `std::string` as well.
--
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] areusch commented on a diff in pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r928473224
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
+static const llvm::CodeGenOpt::Level opt_level = llvm::CodeGenOpt::Aggressive;
+} // namespace defaults
+} // namespace
+
+namespace {
+bool InitializeLLVM() {
+ static std::atomic_flag initialized = ATOMIC_FLAG_INIT;
Review Comment:
why do we need the atomic here? i don't think we typically support multithreaded operation
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
Review Comment:
why'd you need the NOLINT?
##########
src/target/llvm/llvm_scope.h:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef TVM_TARGET_LLVM_LLVM_SCOPE_H_
+#define TVM_TARGET_LLVM_LLVM_SCOPE_H_
+
+#ifdef TVM_LLVM_VERSION
+
+#include <llvm/ADT/ArrayRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/target/target.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+namespace llvm {
+class LLVMContext;
+class MemoryBuffer;
+class Module;
+class TargetMachine;
+} // namespace llvm
+
+namespace tvm {
+namespace codegen {
+
+class LLVMTarget;
Review Comment:
can you add some comments to describe these classes? also it'd be great to explain a little bit of the design rationale here so that folks making changes here understand to preserve the consolidation of flags.
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -121,38 +121,37 @@ std::unique_ptr<CodeGenLLVM> CodeGenLLVM::Create(llvm::TargetMachine* tm) {
}
}
-void CodeGenLLVM::Init(const std::string& module_name, llvm::TargetMachine* tm,
- llvm::LLVMContext* ctx, bool system_lib, bool dynamic_lookup,
- bool target_c_runtime) {
- InitializeLLVM();
- ctx_ = ctx;
- builder_.reset(new IRBuilder(*ctx_));
- module_.reset(new llvm::Module(module_name, *ctx_));
- md_builder_.reset(new llvm::MDBuilder(*ctx_));
+void CodeGenLLVM::Init(const std::string& module_name, LLVMTarget* llvm_target, bool system_lib,
+ bool dynamic_lookup, bool target_c_runtime) {
+ llvm_target_ = llvm_target;
+ llvm::LLVMContext* ctx = llvm_target_->GetContext();
+ builder_.reset(new IRBuilder(*ctx));
+ module_.reset(new llvm::Module(module_name, *ctx));
+ md_builder_.reset(new llvm::MDBuilder(*ctx));
// types
- t_void_ = llvm::Type::getVoidTy(*ctx_);
- t_void_p_ = llvm::Type::getInt8Ty(*ctx_)->getPointerTo(GetGlobalAddressSpace());
- t_int_ = llvm::Type::getInt32Ty(*ctx_);
- t_char_ = llvm::Type::getInt8Ty(*ctx_);
- t_int8_ = llvm::Type::getInt8Ty(*ctx_);
- t_int16_ = llvm::Type::getInt16Ty(*ctx_);
- t_int32_ = llvm::Type::getInt32Ty(*ctx_);
- t_int64_ = llvm::Type::getInt64Ty(*ctx_);
- t_float64_ = llvm::Type::getDoubleTy(*ctx_);
+ t_void_ = llvm::Type::getVoidTy(*ctx);
+ t_void_p_ = llvm::Type::getInt8Ty(*ctx)->getPointerTo(GetGlobalAddressSpace());
+ t_int_ = llvm::Type::getInt32Ty(*ctx);
+ t_char_ = llvm::Type::getInt8Ty(*ctx);
+ t_int8_ = llvm::Type::getInt8Ty(*ctx);
+ t_int16_ = llvm::Type::getInt16Ty(*ctx);
+ t_int32_ = llvm::Type::getInt32Ty(*ctx);
+ t_int64_ = llvm::Type::getInt64Ty(*ctx);
+ t_float64_ = llvm::Type::getDoubleTy(*ctx);
// meta data
md_very_likely_branch_ = md_builder_->createBranchWeights(1 << 20, 1);
md_tbaa_root_ = md_builder_->createTBAARoot("tvm-tbaa");
md_tbaa_alias_set_ = md_builder_->createTBAANode("tvm-alias", md_tbaa_root_);
- this->InitTarget(tm);
+ InitTarget();
}
void CodeGenLLVM::SetFastMathFlag(llvm::FastMathFlags fmf) { builder_->setFastMathFlags(fmf); }
Review Comment:
shall we change this to SetFastMathFlags?
--
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] tqchen commented on pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
tqchen commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204419044
LLVMInstance sounds good
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r931693719
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
Review Comment:
Makes sense. I changed it to `const char*`.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204233776
> [...] perhaps we can change name of LLVMScope to LLVMContext
I'm a bit concerned that it could create confusion given that LLVMContext is name of LLVM type. Let me think a bit, maybe I can come up with a different name.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r928888179
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
+static const llvm::CodeGenOpt::Level opt_level = llvm::CodeGenOpt::Aggressive;
+} // namespace defaults
+} // namespace
+
+namespace {
+bool InitializeLLVM() {
+ static std::atomic_flag initialized = ATOMIC_FLAG_INIT;
Review Comment:
The previous initialization code (in [llvm_common.cc](https://github.com/apache/tvm/pull/12140/files#diff-87ff91bdb4a2845d4ffac8e46ea9648cb72ae71505530a420163202c8906d373L64)) had an atomic flag in it (although in a bit more complex scheme), so I kept it. If you still think it should be removed, let me know---I have no problem changing it.
Edit: llvm_common.cc is not shown in the diff. You need to show it manually for the link to work.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r928913008
##########
src/target/llvm/codegen_llvm.cc:
##########
@@ -121,38 +121,37 @@ std::unique_ptr<CodeGenLLVM> CodeGenLLVM::Create(llvm::TargetMachine* tm) {
}
}
-void CodeGenLLVM::Init(const std::string& module_name, llvm::TargetMachine* tm,
- llvm::LLVMContext* ctx, bool system_lib, bool dynamic_lookup,
- bool target_c_runtime) {
- InitializeLLVM();
- ctx_ = ctx;
- builder_.reset(new IRBuilder(*ctx_));
- module_.reset(new llvm::Module(module_name, *ctx_));
- md_builder_.reset(new llvm::MDBuilder(*ctx_));
+void CodeGenLLVM::Init(const std::string& module_name, LLVMTarget* llvm_target, bool system_lib,
+ bool dynamic_lookup, bool target_c_runtime) {
+ llvm_target_ = llvm_target;
+ llvm::LLVMContext* ctx = llvm_target_->GetContext();
+ builder_.reset(new IRBuilder(*ctx));
+ module_.reset(new llvm::Module(module_name, *ctx));
+ md_builder_.reset(new llvm::MDBuilder(*ctx));
// types
- t_void_ = llvm::Type::getVoidTy(*ctx_);
- t_void_p_ = llvm::Type::getInt8Ty(*ctx_)->getPointerTo(GetGlobalAddressSpace());
- t_int_ = llvm::Type::getInt32Ty(*ctx_);
- t_char_ = llvm::Type::getInt8Ty(*ctx_);
- t_int8_ = llvm::Type::getInt8Ty(*ctx_);
- t_int16_ = llvm::Type::getInt16Ty(*ctx_);
- t_int32_ = llvm::Type::getInt32Ty(*ctx_);
- t_int64_ = llvm::Type::getInt64Ty(*ctx_);
- t_float64_ = llvm::Type::getDoubleTy(*ctx_);
+ t_void_ = llvm::Type::getVoidTy(*ctx);
+ t_void_p_ = llvm::Type::getInt8Ty(*ctx)->getPointerTo(GetGlobalAddressSpace());
+ t_int_ = llvm::Type::getInt32Ty(*ctx);
+ t_char_ = llvm::Type::getInt8Ty(*ctx);
+ t_int8_ = llvm::Type::getInt8Ty(*ctx);
+ t_int16_ = llvm::Type::getInt16Ty(*ctx);
+ t_int32_ = llvm::Type::getInt32Ty(*ctx);
+ t_int64_ = llvm::Type::getInt64Ty(*ctx);
+ t_float64_ = llvm::Type::getDoubleTy(*ctx);
// meta data
md_very_likely_branch_ = md_builder_->createBranchWeights(1 << 20, 1);
md_tbaa_root_ = md_builder_->createTBAARoot("tvm-tbaa");
md_tbaa_alias_set_ = md_builder_->createTBAANode("tvm-alias", md_tbaa_root_);
- this->InitTarget(tm);
+ InitTarget();
}
void CodeGenLLVM::SetFastMathFlag(llvm::FastMathFlags fmf) { builder_->setFastMathFlags(fmf); }
Review Comment:
Done.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r929063619
##########
src/target/llvm/llvm_scope.h:
##########
@@ -0,0 +1,117 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifndef TVM_TARGET_LLVM_LLVM_SCOPE_H_
+#define TVM_TARGET_LLVM_LLVM_SCOPE_H_
+
+#ifdef TVM_LLVM_VERSION
+
+#include <llvm/ADT/ArrayRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/ir/expr.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/target/target.h>
+
+#include <memory>
+#include <string>
+#include <utility>
+#include <vector>
+
+namespace llvm {
+class LLVMContext;
+class MemoryBuffer;
+class Module;
+class TargetMachine;
+} // namespace llvm
+
+namespace tvm {
+namespace codegen {
+
+class LLVMTarget;
Review Comment:
Done.
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204574709
I edited the commit message to use `LLVMInstance` instead of `LLVMScope` in the summary.
--
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] areusch commented on a diff in pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
areusch commented on code in PR #12140:
URL: https://github.com/apache/tvm/pull/12140#discussion_r931657833
##########
src/target/llvm/llvm_scope.cc:
##########
@@ -0,0 +1,368 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#ifdef TVM_LLVM_VERSION
+
+#include "llvm_scope.h"
+
+#include <dmlc/base.h>
+#include <llvm/ADT/ArrayRef.h>
+#include <llvm/ADT/StringRef.h>
+#if TVM_LLVM_VERSION >= 150
+#include <llvm/IR/FMF.h>
+#else
+#include <llvm/IR/Operator.h>
+#endif
+#include <llvm/IR/LLVMContext.h>
+#include <llvm/IR/Metadata.h>
+#include <llvm/IR/Module.h>
+#include <llvm/IRReader/IRReader.h>
+#if TVM_LLVM_VERSION >= 140
+#include <llvm/MC/TargetRegistry.h>
+#else
+#include <llvm/Support/TargetRegistry.h>
+#endif
+#include <llvm/Support/CodeGen.h>
+#include <llvm/Support/ErrorOr.h>
+#include <llvm/Support/Host.h>
+#include <llvm/Support/MemoryBuffer.h>
+#include <llvm/Support/SourceMgr.h>
+#include <llvm/Support/TargetSelect.h>
+#include <llvm/Support/raw_ostream.h>
+#include <llvm/Target/TargetMachine.h>
+#include <llvm/Target/TargetOptions.h>
+#include <tvm/runtime/container/array.h>
+#include <tvm/runtime/container/map.h>
+#include <tvm/runtime/container/optional.h>
+#include <tvm/runtime/container/string.h>
+#include <tvm/runtime/logging.h>
+#include <tvm/runtime/object.h>
+#include <tvm/target/target.h>
+
+#include <atomic>
+#include <sstream>
+#include <string>
+#include <system_error>
+
+namespace tvm {
+namespace codegen {
+
+namespace {
+namespace defaults {
+static const std::string cpu = "generic"; // NOLINT(runtime/string)
Review Comment:
that's fair--i think at global scope the concern is that strings are mutable and live in .data. should we adopt the `const char*` convention here to avoid those pitfalls?
--
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 merged pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic merged PR #12140:
URL: https://github.com/apache/tvm/pull/12140
--
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] tqchen commented on pull request #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
tqchen commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1204140133
Thanks @kparzysz-quic , my guess is that we just need naming and documentation clarifications. e.g. the perhaps we can change name of LLVMScope to LLVMContext to capture the fact that it is " LLVM context creation," and the scoping happens in `With<LLLVMTarget>`
--
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 #12140: [LLVM] Create LLVM scope object for use with LLVM libraries
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on PR #12140:
URL: https://github.com/apache/tvm/pull/12140#issuecomment-1199378156
Pinging all interested parties... Are there any concerns that haven't been addressed?
--
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