Allow register indices in addition to ABI names #52
Conversation
|
@AntonLydike I think you didn't see my PR. I'd appreciate if you could review it. |
riscemu/parser.py
Outdated
| def canonicalize_register_names(args: Tuple[str]) -> Iterable[str]: | ||
| """ | ||
| Canonicalizes register names. | ||
| :param args: A tuple of an instructions arguments | ||
| :return: An iterator over the arguments of an instruction, but with canonicalized register names | ||
| """ | ||
| for arg in args: | ||
| if arg in REG_NAME_CANONICALIZER.keys(): | ||
| yield REG_NAME_CANONICALIZER[arg] | ||
| else: | ||
| yield arg |
There was a problem hiding this comment.
I can't really speak to how this would best fit with the overall architecture of the framework, but I'd recommend the following changes for this function specifically:
| def canonicalize_register_names(args: Tuple[str]) -> Iterable[str]: | |
| """ | |
| Canonicalizes register names. | |
| :param args: A tuple of an instructions arguments | |
| :return: An iterator over the arguments of an instruction, but with canonicalized register names | |
| """ | |
| for arg in args: | |
| if arg in REG_NAME_CANONICALIZER.keys(): | |
| yield REG_NAME_CANONICALIZER[arg] | |
| else: | |
| yield arg | |
| def register_name_for_indexed(indexed_name: str) -> str: | |
| """ | |
| Canonicalizes register name. | |
| :param args: A tuple of an instructions arguments. | |
| :return: An iterator over the arguments of an instruction, but with canonicalized register names. | |
| """ | |
| return REG_NAME_CANONICALIZER.get(arg, arg) |
The function name describes a bit better what it does, I would have expected the canonical name to be the indexed name. The function operates one element at a time, the client can decide whether to get just one name or a tuple or a list at the callsite (tuple(register_name_for_indexed(arg) for arg in args))). There's a built-in method for the pattern of getting a value with a default if it's not in the dictionary.
There was a problem hiding this comment.
oh and I think some examples in the docstring would be good
There was a problem hiding this comment.
Almost @superlopuh , I think you are missing that args is an array here.
But still, I'd simplify the if .. in ... and item getting into a single .get(arg, default) call.
riscemu/parser.py
Outdated
| def canonicalize_register_names(args: Tuple[str]) -> Iterable[str]: | ||
| """ | ||
| Canonicalizes register names. | ||
| :param args: A tuple of an instructions arguments | ||
| :return: An iterator over the arguments of an instruction, but with canonicalized register names | ||
| """ | ||
| for arg in args: | ||
| if arg in REG_NAME_CANONICALIZER.keys(): | ||
| yield REG_NAME_CANONICALIZER[arg] | ||
| else: | ||
| yield arg |
There was a problem hiding this comment.
Almost @superlopuh , I think you are missing that args is an array here.
But still, I'd simplify the if .. in ... and item getting into a single .get(arg, default) call.
This PR is for issue #51.
I have added a function to parse the arguments of an instruction, that is used to canonicalize the register names. This function could be expanded to do other things with the arguments in the future, if needed.
The canonicalization is done using a dict, that is used replace register-index-names with the according ABI-name.
Additionally I have added a file-check, that is just like the "hello world"-check but uses register-indices instead of ABI-names.