MWCC Struct copy improvements #294
Conversation
dbalatoni13
commented
Mar 29, 2025
dbalatoni13
commented
- Adjusted the pattern to make it possible for addresses to not start at zero
- Sometimes the last part uses different registers, this was adjusted in the pattern
- Use a store statement instead of the void function we had till now. This helps with type resolution.
Change the code to test struct type unification
This reverts commit bb4cb49.
| if dest.type.is_struct() or source.type.is_struct(): | ||
| dest.type.unify(source.type) | ||
|
|
||
| # TODO are these four needed? |
There was a problem hiding this comment.
Are these needed here? I copied them from another place (the next four lines)
There was a problem hiding this comment.
Yes, that's needed. Can you break that part out into a shared emit_write function maybe?
| return line | ||
| else: | ||
| return fmt.with_comments( | ||
| line, [f"size 0x{fmt.format_hex(self.commented_size)}"] |
There was a problem hiding this comment.
This causes an extra level of identation, do you have a suggestion maybe how we could get rid of that?
There was a problem hiding this comment.
Eh, it's fine.
It looks like fmt.with_comments gives additional indentation though, we need to fix that somehow.
There was a problem hiding this comment.
Oh, wait, I must have been really tired when reading this, I thought you were referring to Python indentation...
I think the comments probably have to be carried all the way to if_statements somehow. Maybe Statement can grow a method for getting comments from a statement (defaulting to []), and SimpleStatement can check the output of that method?
|
TODO: There's a bug that causes the copy to go onto |
|
We do support addresses not starting at 0 now, but I wonder if that causes false positives on -O4. |
|
It also seems to make no difference whether we manually call |
| @@ -5,35 +5,35 @@ void test(void) { | |||
|
|
|||
| } | |||
|
|
|||
| void test_0(s32 arg0, s32 arg1) { | |||
| M2C_STRUCT_COPY(arg0, arg1, 8); | |||
| void test_0(? *arg0, ? *arg1) { | |||
There was a problem hiding this comment.
We should probably get these to be typed as pointers.
| if dest.type.is_struct() or source.type.is_struct(): | ||
| dest.type.unify(source.type) | ||
|
|
||
| # TODO are these four needed? |
There was a problem hiding this comment.
Yes, that's needed. Can you break that part out into a shared emit_write function maybe?
| size_expr = a.imm(2) | ||
| assert isinstance(size_expr, Literal) | ||
| size = size_expr.value |
There was a problem hiding this comment.
| size_expr = a.imm(2) | |
| assert isinstance(size_expr, Literal) | |
| size = size_expr.value | |
| size = a.imm_value(2) |
| @@ -2075,7 +2076,14 @@ def format(self, fmt: Formatter) -> str: | |||
| ) or isinstance(dest, (ArrayAccess, LocalVar, SubroutineArg)): | |||
| # Known destination; fine to elide some casts. | |||
| source = elide_literal_casts(source) | |||
| return format_assignment(dest, source, fmt) | |||
| line = format_assignment(dest, source, fmt) | |||
| # Used in struct copy statements | |||
There was a problem hiding this comment.
Drop this comment, it's mostly irrelevant it is used and it's confusing what this comment refers to exactly
| return line | ||
| else: | ||
| return fmt.with_comments( | ||
| line, [f"size 0x{fmt.format_hex(self.commented_size)}"] |
There was a problem hiding this comment.
Eh, it's fine.
It looks like fmt.with_comments gives additional indentation though, we need to fix that somehow.
| @@ -5,35 +5,35 @@ void test(void) { | |||
|
|
|||
| } | |||
|
|
|||
| void test_0(s32 arg0, s32 arg1) { | |||
| M2C_STRUCT_COPY(arg0, arg1, 8); | |||
| void test_0(? *arg0, ? *arg1) { | |||
