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<llvm::LLVMContext> 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<llvm::LLVMContext>(CreateNewContext(is_read_only));
  }

  static std::pair<llvm::Module, LLVMScope> ParseIR(std::string ir) {   // same 
for LoadIR
    auto ctx = std::shared_ptr<llvm::LLVMContext>(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<llvm::LLVMContext> ctx, const Target& target);
  std::shared_ptr<llvm::LLVMContext> 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: <apache/tvm-rfcs/pull/83/c1179203...@github.com>

Reply via email to