zxybazh commented on a change in pull request #8823:
URL: https://github.com/apache/tvm/pull/8823#discussion_r694504182
##########
File path: python/tvm/target/target.py
##########
@@ -510,23 +548,27 @@ def validate_hvx_length(codegen_hvx, sim_args):
+ cpu_attr["post"]
)
- return sim_cpu + " " + validate_hvx_length(hvx, sim_args)
+ return sim_cpu + " " + validate_hvx_length(hvx, sim_options)
+
+ # LLVM options string
+ def create_llvm_options(cpu_ver, args): # pylint: disable=unused-argument
Review comment:
Can we simply pass `llvm_options` as the argument here, since we use
nothing out of that scope. `args` is a bit more vague.
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
+
+ # Warn about obsolete parameter names.
+ if args.get("sim_args"):
+ msg = "The keyword parameter 'sim_args' is deprecated, use
'sim_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"sim_options": args["sim_args"]})
+ if args.get("llvm_args"):
+ msg = "The keyword parameter 'llvm_args' is deprecated, use
'llvm_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"llvm_options": args["llvm_args"]})
+
+ # LLVM target string
+ def create_llvm_target(cpu_ver, args):
Review comment:
Can we use `cpu_ver` and `hvx` as argument here?
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
+
+ # Warn about obsolete parameter names.
+ if args.get("sim_args"):
+ msg = "The keyword parameter 'sim_args' is deprecated, use
'sim_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"sim_options": args["sim_args"]})
+ if args.get("llvm_args"):
+ msg = "The keyword parameter 'llvm_args' is deprecated, use
'llvm_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"llvm_options": args["llvm_args"]})
Review comment:
Ditto, if we remove `args`, we only need to update the `llvm_options`
variable.
##########
File path: python/tvm/target/target.py
##########
@@ -510,23 +548,27 @@ def validate_hvx_length(codegen_hvx, sim_args):
+ cpu_attr["post"]
)
- return sim_cpu + " " + validate_hvx_length(hvx, sim_args)
+ return sim_cpu + " " + validate_hvx_length(hvx, sim_options)
+
+ # LLVM options string
+ def create_llvm_options(cpu_ver, args): # pylint: disable=unused-argument
+ """ Create LLVM options string. """
+
+ llvm_options = args["llvm_options"]
- # LLVM string
- def create_llvm(llvm_args):
# TVM's option parser doesn't allow '=' in values, but '=' can
# appear in LLVM flags. Replace it with '@', since it's unlikely
# that '@' will be used in another context.
- if llvm_args is None or len(llvm_args.replace(" ", "")) == 0:
+ if llvm_options is None or len(llvm_options.replace(" ", "")) == 0:
Review comment:
You might want to use `isspace()` function here.
##########
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:
IMHO, we can use the following function signature so that the arguments
are clear, and deprecate `sim_args` and `llvm_args` later.
```suggestion
def hexagon(cpu_ver="v66", *, sim_options=None, llvm_options=None, hvx=128,
sim_args=None, llvm_args=None):
```
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
+
+ # Warn about obsolete parameter names.
+ if args.get("sim_args"):
+ msg = "The keyword parameter 'sim_args' is deprecated, use
'sim_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"sim_options": args["sim_args"]})
+ if args.get("llvm_args"):
+ msg = "The keyword parameter 'llvm_args' is deprecated, use
'llvm_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"llvm_options": args["llvm_args"]})
+
+ # LLVM target string
+ def create_llvm_target(cpu_ver, args):
+ """ Create LLVM target string. """
- # Target string
- def create_target(cpu_ver):
target = " -mtriple=hexagon"
mcpu = " -mcpu=hexagon" + cpu_ver
- mattr = ""
- # HVX enable
- if hvx:
- mattr = " -mattr=+hvx" + cpu_ver + ",+hvx-length" + str(hvx) + "b"
- return target + mcpu + mattr
-
- # Simulator string
- def create_sim(cpu_ver, sim_args):
- def validate_hvx_length(codegen_hvx, sim_args):
- if sim_args and "--hvx_length" in sim_args:
+
+ # Process the options that affect target features and return the
+ # target feature string.
+ def create_target_features(args):
+ tfs = []
+ if args["hvx"] > 0:
+ valid_hvx = [0, 64, 128]
+ if not args["hvx"] in valid_hvx:
+ raise ValueError("Invalid hvx value, should be one of " +
str(valid_hvx))
+ tfs += ["+hvx" + cpu_ver, "+hvx-length" + str(args["hvx"]) +
"b"]
+ else:
+ tfs += ["-hvx"]
+ return "-mattr=" + ",".join(tfs) if tfs else ""
+
+ return target + mcpu + " " + create_target_features(args)
+
+ # Simulator options string
+ def create_sim_options(cpu_ver, args):
Review comment:
Can we use `hvx` and `sim_options` as the argument here?
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
+
+ # Warn about obsolete parameter names.
+ if args.get("sim_args"):
+ msg = "The keyword parameter 'sim_args' is deprecated, use
'sim_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"sim_options": args["sim_args"]})
Review comment:
If we remove `args`, we only need to update the `sim_options` variable.
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
Review comment:
As commented below, IMHO we don't need the `args` variable. We just need
to make sure the arguments have default value in function signature and the
corresponding variable(i.e., `hvx`, `sim_options`, `llvm_options`) is given to
the functions below.
##########
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):
"""Returns a Hexagon target.
Parameters
----------
- cpu_ver : str
+ cpu_ver : str (default: "v66")
CPU version used for code generation. Not all allowed cpu str
will be valid, LLVM will throw an error.
- sim_args : str or list of str
+
+ Recognized keyword parameters
+ -----------------------------
+ hvx : int (default: 128)
+ Size of HVX vector in bytes. Value of 0 disables HVX codegen.
+ sim_options : str or list of str (default: None)
User defined sim arguments. CPU version defaults to cpu_ver.
Otherwise, separate versions are used for codegen and sim. Not
all allowed cpu strings will be valid, simulator will throw an
error if invalid. Does not affect codegen.
- llvm_args : str or list of str
+ llvm_options : str or list of str (default: None)
User defined compiler arguments.
- hvx : int
- Size of hvx register. Value of 0 indicates disabled hvx.
"""
+
+ # Some of the target parameters correspond to target kind attributes
+ # listed in src/target/target_kind.cc. For those parameters, their
+ # names follow the attribute names with the exception of '_' being used
+ # in place of '-'.
+
# Example compiler arguments
# llvm -mtriple=hexagon -mcpu=hexagonv66 -mattr=+hvxv66,+hvx-length128b
# Check for valid codegen cpu
- valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t"]
+ valid_hex = ["v60", "v62", "v65", "v66", "v67", "v67t", "v68"]
try:
cpu_ver = cpu_ver[cpu_ver.index("v") :].lower()
- assert 3 <= len(cpu_ver) <= 4
+ assert cpu_ver in valid_hex
except:
msg = "{} is not a valid Hexagon version\nvalid versions include {}"
raise ValueError(msg.format(cpu_ver, valid_hex)) from None
- assert hvx in [0, 64, 128]
+ args = {
+ "hvx": 128,
+ "sim_options": None,
+ "llvm_options": None,
+ }
+ args.update(kwargs)
+
+ # Warn about obsolete parameter names.
+ if args.get("sim_args"):
+ msg = "The keyword parameter 'sim_args' is deprecated, use
'sim_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"sim_options": args["sim_args"]})
+ if args.get("llvm_args"):
+ msg = "The keyword parameter 'llvm_args' is deprecated, use
'llvm_options' instead"
+ warnings.warn(msg, stacklevel=2)
+ args.update({"llvm_options": args["llvm_args"]})
+
+ # LLVM target string
+ def create_llvm_target(cpu_ver, args):
+ """ Create LLVM target string. """
- # Target string
- def create_target(cpu_ver):
target = " -mtriple=hexagon"
mcpu = " -mcpu=hexagon" + cpu_ver
- mattr = ""
- # HVX enable
- if hvx:
- mattr = " -mattr=+hvx" + cpu_ver + ",+hvx-length" + str(hvx) + "b"
- return target + mcpu + mattr
-
- # Simulator string
- def create_sim(cpu_ver, sim_args):
- def validate_hvx_length(codegen_hvx, sim_args):
- if sim_args and "--hvx_length" in sim_args:
+
+ # Process the options that affect target features and return the
+ # target feature string.
+ def create_target_features(args):
Review comment:
Ditto, `hvx` seems enough as argument.
--
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]
