kparzysz-quic commented on a change in pull request #8823:
URL: https://github.com/apache/tvm/pull/8823#discussion_r695037760
##########
File path: python/tvm/target/target.py
##########
@@ -413,79 +413,117 @@ def bifrost(model="unknown", options=None):
return Target(" ".join(["opencl"] + opts))
-def hexagon(cpu_ver="v66", sim_args=None, llvm_args=None, hvx=128):
+def hexagon(cpu_ver="v66", **kwargs):
Review comment:
Thanks @zxybazh for the detailed feedback. You are making valid points,
but I think I should share more context of why the code looks like this.
In our downstream repository we have a few more options for the Hexagon
target that, for one reason or another, we cannot yet make public. We have
reorganized the `tvm.target,hexagon()` so that we don't need to change the
interface every time we add a new flag---this is the reason to use the keyword
arguments instead of explicit named arguments.
The code downstream has diverged from the upstream version, and we would
like to keep these two close to reduce the risk of conflicts (when integrating
upstream code to our downstream repo). Essentially, what this patch is, is the
downstream code with the non-public parts deleted. This is why several places
look like they could be simplified, but if we did simplify them upstream, it
would create divergence again.
I'd like to ask that this code is accepted without removing `args` since the
downstream code has more fields in it, and is organized around handling the
contents of the target configuration represented by `args`.
--
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]
