FranckQC commented on a change in pull request #9482:
URL: https://github.com/apache/tvm/pull/9482#discussion_r797943017



##########
File path: python/tvm/tir/transform/transform.py
##########
@@ -310,6 +310,15 @@ def BF16TypeLowering():
     """
     return _ffi_api.BF16TypeLowering()  # type: ignore
 
+def CommonSubexprElim(enable_cse_tir: bool = True):

Review comment:
       Thanks for this remark! I am a little bit confused about things 
regarding activating/deactivating a pass at the moment.
   If I understand well, there is two ways to disable a pass :
   
   1) In driver_api.cc, we have the registration of options:
   ```
   // Register build pipeline related options
   [...]
   TVM_REGISTER_PASS_CONFIG_OPTION("tir.disable_vectorize", Bool);
   TVM_REGISTER_PASS_CONFIG_OPTION("tir.disable_storage_rewrite", Bool);
   TVM_REGISTER_PASS_CONFIG_OPTION("tir.is_entry_func", Bool);
   TVM_REGISTER_PASS_CONFIG_OPTION("tir.add_lower_pass", 
Array<Array<ObjectRef>>);
   TVM_REGISTER_PASS_CONFIG_OPTION("tir.debug_keep_trivial_loop", Bool);
   ```
   
   Then in Array<tvm::transform::Pass> CreatePassList(), we get these booleans 
from the `config`:
   ```
     bool disable_vectorize = 
pass_ctx->GetConfig<Bool>("tir.disable_vectorize", Bool(false)).value();
     bool disable_storage_rewrite =
         pass_ctx->GetConfig<Bool>("tir.disable_storage_rewrite", 
Bool(false)).value();
     bool instrument_bound_checkers =
         pass_ctx->GetConfig<Bool>("tir.instrument_bound_checkers", 
Bool(false)).value();
     bool disable_cse_tir = pass_ctx->GetConfig<Bool>("tir.disable_cse_tir", 
Bool(false)).value();
   ```
   
   And finally when we push the passes into the `pass_list`, we use the boolean 
that we got.
   For instance:
   ```
   [...]
    pass_list.push_back(tir::transform::VectorizeLoop(!disable_vectorize));
    pass_list.push_back(tir::transform::CommonSubexprElimTIR(!disable_cse_tir));
   ```
   
   That makes sense to me.
   Regarding what you said about the list of disabled pass, I assume that when 
a pass is mentioned in the disabled pass list, the context will get this 
information (somehow), and so the information will later flow through the 
GetConfig() showed above.
   Am I correct on this?
   
   2) In addition, some passes, like Vectorize, expose a boolean to the python 
interface for activating/deactivating them.
   In transform.py:
   ```
   def VectorizeLoop(enable_vectorize: bool = True):
       """Lower vectorization loops.
       [...]
       return _ffi_api.VectorizeLoop(enable_vectorize)  # type: ignore
   ```
   
   I decided to do the same for the CSE. I though that would make it easier to 
disable the pass from the Python bindings, for instance for doing some 
experiments. If I remember well I was told that exposing the boolean would be 
useful (perhaps for the end-to-end compilation?).
   Why is it done for Vectorize exactly? Should I keep it for the CSE?
   
   Side note :
   The pass LoopPartition is quite weird, as its boolean for 
activation/deactivation is passed directly to the `CreatePassList()` method:
   `Array<tvm::transform::Pass> CreatePassList(bool disable_loop_partition)`
   Why doesn't it get its boolean from the config, like all the other passes do?
   
   Any precision would be very much appreciated :) Thanks!




-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to