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

cmd/internal/obj/riscv: riscv64 compressed instructions #47560

Open
4a6f656c opened this issue Aug 5, 2021 · 4 comments
Open

cmd/internal/obj/riscv: riscv64 compressed instructions #47560

4a6f656c opened this issue Aug 5, 2021 · 4 comments
Labels
arch-riscv Issues solely affecting the riscv64 architecture. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@4a6f656c
Copy link
Contributor

4a6f656c commented Aug 5, 2021

The RISC-V architecture has support for compressed instructions - these are effectively a subset of the standard 4 byte long instructions, which fit in 2 bytes. These compressed instructions have limited functionality, however require half the code size.

Adding support to the assembler is relatively straight forward and from a code generation perspective, things work as expected and the binary runs. However, I keep hitting problems relating to the pcdatavalue tables while building toolchain3 - this is in the form of deadlock loops while generating tracebacks for inline functions, "runtime: invalid pc-encoded table" from symtab.go while handling async pre-empt, etc.

These all seems to suggest that the various PCDATA tables are wrong/broken, however as far as I can tell p.Pc is being set correctly in cmd/internal/obj/riscv. I would also expect that we handle variable instruction sizes (and variable p.Pc steps), given CISC architectures like x86.

Overall it seems like I'm missing something obvious - should things work if p.Pc is set correctly on a per *obj.Prog basis (even with multiple machine instructions with various widths being generated)?

Any suggestions regarding what else I should check?

(will mail out the work in progress code shortly)

@4a6f656c
Copy link
Contributor Author

4a6f656c commented Aug 5, 2021

/cc @cherrymui

@gopherbot
Copy link

Change https://golang.org/cl/340151 mentions this issue: cmd/internal/obj/riscv: compressed instruction support for riscv64

@randall77
Copy link
Contributor

You probably want to look at MinLC in cmd/internal/sys/arch.go. The PCs in pcln tables are expressed as multiples of this constant.

@seankhliao seankhliao added arch-riscv Issues solely affecting the riscv64 architecture. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 7, 2021
@4a6f656c
Copy link
Contributor Author

4a6f656c commented Aug 8, 2021

@randall77 thanks - turns out it was MinLC and PCQuantum - adjusting those, along with a change to runtime.goexit (smaller NOP slots) and we have working code.

@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-riscv Issues solely affecting the riscv64 architecture. NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants