Re: [apache/tvm-rfcs] [RFC] Create LLVM scope class for use with LLVM libraries (PR #83)
Thanks @kparzysz-quic . Sorry for the delayed reply since we are taking break
here.
Overall I like the direction we are going. Just to figure out the spectrum of
possible APIs
My main question is how are we going to interact with multiple ParseIR calls.
Some example would be helpful. For example, is it OK for us to have nested
LLVMScope
```c++
void Example() {
LLVMScope scope1(target);
{
// what is the effect here, seems mod_data1 is immediate in scope
auto mod_data1 = LLVMScope::ParseIR(name);
}
}
```
I also wonder if there is a way to "defer" the scope initialization. e.g. can
LLVMModule be created, stored in the LLVMScope, but the target does not take
in-effect until we enter the scope. We call InitializeLLVM in the constructor
of LLVMTarget, but do other things like option setting in the enter
stage.Something like
```c++
void Example() {
LLVMTarget target1(target);
auto mod_data = LLVMTarget::ParseIR(name);
// enter target1 scope
With scope1(target1);
{
// entering target in mod_data
With scope2(mod_data);
}
}
```
I think the main question is how coupled the operations related to LLVM are.
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/83#issuecomment-1178983089
You are receiving this because you are subscribed to this thread.
Message ID:
Re: [apache/tvm-rfcs] [RFC] Name mangling in IRModules (PR #84)
> Regarding the name_hint being a final name. I am too of this opinion (it > should be a final name) and believe that it already works this way. If I am > not wrong, the only edge case is where we use the global_symbol attribute > instead. We should define "final name". In this context, it is helpful to look at existing practices. This is a topic related to linking in most compilers, and they are closely related to the linkage type. - For a function that have ExternalLinkage, the name **cannot** change, and whatever name being supplied is the final name, this is because the users of the library needs to be able to refer to the function using the provided name. - For a function that have PrivateLinkage, the name can and should be able to change, this is because when we link multiple functions together, there can be two private functions that comes with the same name, as a result, the conflicted function can be renamed. Private functions are only referred within the module, so we can afford to rewrite all the callers of the function to the new name. - During optimization, if a function with PrivateLinkage is not called by any functions, we can safely remove that function, but we cannot do that for ExternalLinkage. There are more linkage types in LLVM but this two types roughly are sufficient for our discussion purposes. Now come back to our context, we roughly need two things: - N0: We need an IR node to uniquely identify a function, storing hints to the name, so new names can be generated base on the hint, but not necessarily pinning down the name (due to the need to support PrivateLinkage). That is `GlobalVar`. - N1: We need a way to uniquely specify the name, or specify that the linkage type of the function is external. Note that N0 means we cannot simply enforce a `name` or `name_hint` field to be final, it have to be a combination with another attribute that derives the linkage type. So our current convention is: - When a function have `global_symbol` attribute, it specifies the "final name"(as in symbol in the symbol tabel), and indicate that the LinkageType is External - When a function does not have `global_symbol` attribute, it means the LinkageType is private, at some time point, the compiler can choose to pick the final name, by attaching global symbol based on the `name_hint` of the function. But that should be deferred to as late as possible so we can perform optimizations like eliminate dead functions. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/84#issuecomment-1178996339 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm] [ci][docker] Use RFC image tags only (PR #11938)
It has been a while since this PR was updated, @Mousius @areusch please leave a review or address the outstanding comments. @driazati if this PR is still a work in progress, please [convert it to a draft](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft) until it is ready for review. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm/pull/11938#issuecomment-1179123573 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Create LLVM scope class for use with LLVM libraries (PR #83)
Both `LoadIR` or `ParseIR` take an optional pre-existing `LLVMContext`, and
return a pair `{llvm::Module, LLVMScope}`, where the scope object in the pair
is newly constructed, and contains the given `LLVMContext` (or create a new one
if none was given). In the first call to `ParseIR`, the caller can take the
second element of the pair and just use it as the scope. In a subsequent call,
the caller can simply discard the second element, which will then be
automatically destroyed.
An example of calling `ParseIR` or `LoadIR` when a LLVM scope already exists is
in [`HandleImport` in
codegen_llvm.cc](https://github.com/apache/tvm/pull/11933/files#diff-f61b04b100f5145f2681340c81d3f2af221239594ed01e2e24896522329ce92cR275).
You bring up a good point about when the scope "takes effect", or binds to
LLVM. I propose that the scope becomes active when a new `LLVMContext` is
created (i.e. the scope is really determined by `LLVMContext`, not by the
`LLVMScope`). We could also define "takes effect" to mean that locks (or
failure-to-create object) could be acquired. That would mean that `ParseIR`
and `LoadIR` return an active scope. Also, if they are called with a context
argument (meaning that they are called from an active scope), they wouldn't
need to create a new context, and so they wouldn't block (if we were to add
locks). In such case they would return another scope object with the same
`LLVMContext`. This shouldn't cause any problems even if both scope objects
were used, but could make the code confusing to read. If we follow the
convention that the "extra" scope is to be ignored, this would avoid the
confusion. There may be details to be sorted out about releasing locks, etc,
but those are implementation details.
_Side note: Ideally, prior to the activation of the scope, none of LLVM code or
data types would be exposed to the user, but this RFC doesn't try to go that
far. The main concern here is a platform on which saving/restoring LLVM flags
could be done._
I also suggest to create two classes: `LLVMScope` and `LLVMTarget`. The
`LLVMTarget` would contain all the LLVM target flags and options, while
`LLVMScope` would deal with activation, `ParseIR` and `LoadIR`. The
`LLVMTarget` object could be passed around, copied, etc (without concerns about
locking or activating anything), and its purpose would be to have all the
target flags/options in one place, and it could only exist once scope has been
activated.
Something like
```C++
class LLVMTarget {
public:
LLVMTarget(const LLVMTarget&); // copy ctor, etc.
private:
friend class LLVMScope;
LLVMTarget(ctx, values extracted from tvm::Target);
std::shared_ptr ctx_;// set via the private ctor,
passed from LLVMContext
llvm::TargetMachine *tm;
...
};
class LLVMScope {
public:
LLVMScope(const Target& target) {
// Parse the target into individual attributes, e.g. mtriple, etc. This
would allow users
// to create LLVMScope from Target and modify it before creating LLVMTarget.
//
// If the target has non-empty list of LLVM flags, it would be considered
"modifying"
// the global state, otherwise it would be "read-only". This would allow
creating
// multiple "read-only" concurrent scopes, but only one "modifying" one.
}
void activate() {
ctx_ = std::shared_ptr(CreateNewContext(is_read_only));
}
static std::pair ParseIR(std::string ir) { // same
for LoadIR
auto ctx = std::shared_ptr(CreateNewContext(false
/*assume modifying*/));
llvm::Module m = llvm::parse_string_etc(ctx, ...)
LLVMScope scp(ctx, get_target_string(m)); // create an already active
scope
return {m, scp};
}
LLVMTarget getTargetInfo() {
ICHECK(ctx_ != nullptr) << "must activate first";
...
}
private:
static llvm::LLVMContext* CreateNewContext(bool is_read_only) {
// Create context with appropriate locking
}
LLVMScope(std::shared_ptr ctx, const Target& target);
std::shared_ptr ctx_;
};
```
--
Reply to this email directly or view it on GitHub:
https://github.com/apache/tvm-rfcs/pull/83#issuecomment-1179203419
You are receiving this because you are subscribed to this thread.
Message ID:
Re: [apache/tvm-rfcs] [RFC] Create LLVM scope class for use with LLVM libraries (PR #83)
To address the second question: we shouldn't allow nesting of LLVM scopes. The reason is that if the target doesn't specify any command line flags, the assumption is that it will rely on LLVM's defaults. If we were to nest scopes, and the outer scope did modify flags, then the inner scope (without flags) would not have a way of knowing that the global state has been altered. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/83#issuecomment-1179206731 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Create LLVM scope class for use with LLVM libraries (PR #83)
Just to followup, why does ParseIR require activation of a scope, or is it possible that ParseIR returns a tuple of Target and Module, where activation is done separately. I am asking this mainly to see if we can get an explicit With style API -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/83#issuecomment-1179234699 You are receiving this because you are subscribed to this thread. Message ID:
Re: [apache/tvm-rfcs] [RFC] Create LLVM scope class for use with LLVM libraries (PR #83)
> Just to followup, why does ParseIR require activation of a scope, or is it > possible that ParseIR returns a tuple of Target and Module, where activation > is done separately. The reason is that `ParseIR` needs to make calls to LLVM functions to create the `llvm::Module`. -- Reply to this email directly or view it on GitHub: https://github.com/apache/tvm-rfcs/pull/83#issuecomment-1179274109 You are receiving this because you are subscribed to this thread. Message ID:
