[GitHub] [tvm-rfcs] kparzysz-quic commented on a diff in pull request #83: [RFC] Encapsulate LLVM target for use with LLVM libraries

[GitBox <gi...@apache.org>]

kparzysz-quic commented on code in PR #83:
URL: https://github.com/apache/tvm-rfcs/pull/83#discussion_r910298381


##########
rfcs/0080-llvm-target.md:
##########
@@ -0,0 +1,174 @@
+- Feature Name: Encapsulate LLVM target for use with LLVM libraries
+- Start Date: May 13, 2022
+- RFC PR: [apache/tvm-rfcs#0000](https://github.com/apache/tvm-rfcs/pull/0000)
+- GitHub Issue: None
+
+# Summary
+
+1. Enapsulate all information related to a compilation target in LLVM into a
+single object `LLVMTarget`. Make creation of this object a prerequisite
+for using any LLVM facilities (e.g. optimizations, code generation, etc.).
+
+2. Extend the `llvm` target in TVM to contain LLVM flags, use `LLVMTarget`
+to save/restore LLVM's command line options based on the flags contained
+in the `llvm` target.
+
+# Motivation
+
+For more details, see [discussion](https://discuss.tvm.apache.org/t/modularizing-llvm-codegen-jit/12764)
+on discourse.
+
+The main issue with using statically linked LLVM libraries is that the LLVM
+code has, and depends on a global state. A specific (and most problematic)
+example of that are command line flags (implemented via `cl::opt` in LLVM).
+Many LLVM components use them to tune their behavior, provide debugging or
+tracing facilities, or simply as on/off switches. In LLVM sources they are
+global variables, and once set they maintain their values.
+
+Since TVM uses LLVM to generate code for multiple different targets, each
+specific code generator in TVM may want to use its own set of tuning flags
+without affecting code generation for other targets. Similarly, using debug
+flag to investigate an issue with a code generation should not affect
+unrelated uses of LLVM.
+
+Luckily, LLVM does provide an interface into the command option registry,
+which allows clients to query and set the values of these options. TVM
+could utilize this to set and restore LLVM's options for the duration of
+code generation for each target. This could be done by having a single
+"entry point" into LLVM, that each LLVM client would need to use. This
+RFC proposes a class that would serve as such entry point.
+
+Since uses of LLVM in TVM are tied to compilation targets, having a common
+class describing a compilation target in LLVM's terms would serve two
+purposes
+1. Would be a unified bridge between the `llvm` target in TVM and target
+representation in LLVM, and
+2. Would be the "entry point" into LLVM described above.
+
+# Guide-level explanation
+
+The idea of this RFC is to implement a common class `LLVMTarget` for all
+LLVM-based targets. Objects of this class would be constructed from TVM's
+`Target`, specifically from `Target` objects for `llvm` target.
+The objects would contain the LLVM representations of the information
+represented by the TVM target, i.e. objects used by LLVM such as
+`TargetMachine`. In addition to translating the target data from TVM format
+to LLVM format, this object would also query the current state of relevant
+LLVM command line options, set then to new values, and restore the original
+values on exit (the indent is to use constructor/desctructor for this).
+
+A typical use would follow this pattern:
+```C++
+{
+  // Let's see the LLVM IR and MIR after each transformation, i.e. use
+  // -print-after-all in codegen.
+  my_target = Target("llvm -mtriple myarch-unknown-elf -llvm-options=print-after-all");
+  LLVMTarget llvm_target(my_target);

Review Comment:
   I think renaming `LLVMTarget` to `LLVMScope` is probably technically a better approach at the moment, although personally I like `With` a bit better (since it resembles the Python idiom, both visually and functionally).
   
   The problem is when we want to initialize the LLVM scope by deserializing `llvm::Module` (either from file, or from a string).  Since the target string is stored in the module, the `llvm::Module` needs to be created first (which means that it would be created before the scope object itself).  To hide the gap between creating `llvm::Module`, and creating the scope object, the `LLVMTarget` (using old naming) implements both steps in a single function call.  This call then returns both, the `llvm::Module` and the scope object.
   
   The issue with `With` in the above situation is that it doesn't apply very nicely to this case.  The return type (currently named `ModuleData` (for a lack of a better idea), is
   ```C++
   std::pair<std::unique_ptr<llvm::Module>, std::unique_ptr<LLVMTarget>>
   ```
   With `With`, it would become
   ```C++
   std::pair<std::unique_ptr<llvm::Module>, std::unique_ptr<With<LLVMTarget>>>
   ```
   which separates the "with" from its scope.  This is not formally wrong, but loses the visual impact of "with".



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