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

Various fixes & minor improvements #16

Merged
merged 4 commits into from
Nov 16, 2021
Merged

Conversation

Amanieu
Copy link
Contributor

@Amanieu Amanieu commented Nov 13, 2021

  • Fixed a bug from Adjust Operand encoding #13 with PRegs with index > 32
  • Removed regs from MachineEnv since it's not actually needed.
  • Return safepoint_slots as Allocations instead of SpillSlots .
    • This enables us to support reftype vregs in register locations in the
      future.
  • Removed an unused clobbers vector.

It isn't exactly clear what purpose it serves.
@Amanieu Amanieu changed the title Various fixed & minor improvements Various fixes & minor improvements Nov 13, 2021
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks reasonable, thanks! The generalization to reftypes-in-Allocation is good and will eventually be useful (for use-cases where the runtime supports it). IIUC, this hasn't changed where refs are actually placed at safepoints though, right? It might be worth adding a note in the safepoint_slots doc-comment noting that for now, all values will be in stackslots, until a future option is added (per-safepoint or for the whole function).

With that change, 👍 from me!

Also, to ensure complete explicitness and clarity, re: the relicensing that is ongoing in #7 currently, you are fine with this contribution being under the new license as well, yes?

Thanks!

This enables us to support reftype vregs in register locations in the
future.
@Amanieu
Copy link
Contributor Author

Amanieu commented Nov 16, 2021

I've updated the doc comment for safepoint_slots.

And yes, I'm fine with relicensing this as described in #7.

@cfallin
Copy link
Member

cfallin commented Nov 16, 2021

Thanks!

@cfallin cfallin merged commit 9774e97 into bytecodealliance:main Nov 16, 2021
@Amanieu Amanieu deleted the misc branch November 24, 2023 04:42
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

Successfully merging this pull request may close these issues.

2 participants