tkonolige commented on pull request #10493:
URL: https://github.com/apache/tvm/pull/10493#issuecomment-1062029215


   @kparzysz-quic I am trying to be a good contributor here by fixing the 
underlying problem of not using logging correctly instead of just putting in 
place a stop-gap solution. To be specific, `#define TVM_LOG_CUSTOMIZE` is set 
in a couple of `.cc` files when it needs to be set for all files in TVM (and 
anything using TVM header files). I'd prefer not to have a stop-gap solution as 
they tend to sit around and not get fixed.
   
   The reason that `include(Hexagon.cmake)` needs to be moved is because I do 
not want to propagate the `USE_HEXAGON_LOGGING` flag to anything that includes 
TVM as a cmake dependency (I want to use `target_compile_definitions` instead 
of `add_definitions`). This necessitates a couple of changes because 
`Hexagon.cmake` was written in a way that sets a lot of global state. Ideally, 
all `include(*.cmake)` would occur after the targets are create (and indeed 
some `include`s are already moved after the targets are created).
   
   > If some other target wants to customize logs as well, their include would 
need to be moved too. Not only that, but the corresponding cmake file would 
need to be modified quite a bit to use the actual targets (like 
tvm_runtime_objs) instead of variables.
   
   Yes, they will. And all our files should be moved to use actual targets 
instead of setting variables. It's just a lot of work to move them all, so I 
have been doing it piece by piece.


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