Merge Retire_Failure into ExecutionResult #884
Conversation
|
Another alternative is the second commit in #777. I'm okay with either. |
|
I think that just renames them rather than merging them. |
|
Yeah, it's an alternative to merging, since it keeps the generic top-level union. I think the 'Exec_{Complete,Incomplete}' is more explicit if also more verbose, which I slightly prefer. But like I said, I'm okay with either approach. |
For LLVM I use https://github.com/spacedentist/spr which works nicely -- the downside is that you need to have write access to the repository you are contributing to (to create This works pretty well but adds lots of extra branches to the repository. |
Timmmm
added
the
will be merged
There's really no reason to have two levels of unions here so I added `Retire_Success` into `Retire_Failure` and then used that directly as `ExecutionResult`. This is definitely simpler and it also removes the weird case of WFI which was included in `Retire_Failure` when it isn't really failing to retire (especially if `wfi_is_nop()` is true). I left the `RETIRE_SUCCESS` to make the commit smaller, but it does mean there is some stylistic inconsistency. We can fix that with a simple search and replace in future.
Timmmm
force-pushed
the
user/timh/retire_failure
branch
from
69dc222 to
179c334
Compare
Note: This is built on #883 so only look at the second commit (I wish Github supported this workflow nicely!)
There's really no reason to have two levels of unions here so I added Retire_Success into Retire_Failure and then used that directly as ExecutionResult. This is definitely simpler and it also removes the weird case of WFI which was included in Retire_Failure when it isn't really failing to retire (especially if wfi_is_nop() is true).
I left the RETIRE_SUCCESS to make the commit smaller, but it does mean there is some stylistic inconsistency. We can fix that with a simple search and replace in future.