Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Decompiler] Support stack spills #368

Closed
5 tasks done
water111 opened this issue Apr 18, 2021 · 2 comments
Closed
5 tasks done

[Decompiler] Support stack spills #368

water111 opened this issue Apr 18, 2021 · 2 comments

Comments

@water111
Copy link
Collaborator

water111 commented Apr 18, 2021

Stack spills in GOAL are rare, but they do happen. They always involve loads/stores from the stack, unlike stack allocated structures, which only get pointers to stuff on the stack. GOAL seems pretty stupid with its stack spills and I'm hoping it doesn't reuse stack slots with variables of different types.

So far I've seed quadword/doubleword and word (float only).

It seems like it would be a huge amount of effort to support stack-spilled variables "fully". I think it's important that stack variable code "works" in the OpenGOAL compiler with no modifications, but it seems okay to allow messier code.

I'm imagining that we would declare all stack variables at the top of the function with local-vars and then have a GetOrSetStackVariableElement that represents the load/stores from the stack.

It seems like it's possible to ahead-of-time figure out the layout of the stack variables and not require any user input for this. That said, if I am wrong about the simpler stack variable allocator, we will need a more complicated strategy for stack variable types.

Implementation plan:

  • Analysis pass to scan all instructions, looking for stack spill load/stores and build stack variable map.
  • Type propagation on stack variable slots. Try to determine if this can be done automatically, or if GOAL reused stack slots.
  • Recognize and convert these instructions into GetOrSetStackVariableElement
  • Insert local-vars for these, at the top of the function
  • Allow the user to rename these through local-vars config file.
@water111 water111 linked a pull request Apr 25, 2021 that will close this issue
@water111 water111 removed a link to a pull request Apr 25, 2021
@water111
Copy link
Collaborator Author

#382 Supports most of this. There are two known limitations:

  • Casts may need some small tweaks (variable vs. register type)
  • Only tested for quadword spills of GPRs. This seems to be the default.

@water111
Copy link
Collaborator Author

Stack spills now work for GPRs and FPRs.
Taking an address of stack spilled variables still needs to be added (#471)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant