search

mmcloughlin / avo

Generate x86 Assembly with Go
BSD 3-Clause "New" or "Revised" License
2.73k stars 89 forks source link

PCALIGN seems to be missing #419

Open egonelbre opened 10 months ago

egonelbre commented 10 months ago

Go will soon support PCALIGN pseudo instruction for amd64.

https://tip.golang.org/doc/asm#special-instructions

mmcloughlin commented 10 months ago

Thanks for the issue. Do you think it would be sufficient to just append this to the instruction set (probably with the opcodesextra mechanism)? Or is there some reason we'd need to handle these pseudo instructions differently?

mmcloughlin commented 10 months ago

PCALIGN is handled by the architecture-independent part of the Go assembler:

https://github.com/golang/go/blob/go1.21.6/src/cmd/asm/internal/asm/asm.go#L336

There are other pseudo-ops defined here, for example FUNCDATA and PCDATA mentioned in #144.

At the moment I can't think of a reason not to just treat this as another ISA instruction. Of course it's still a goal for avo to support ARM #189, in which case these pseudo-ops would need to be pulled out into an architecture-independent set of instructions. But we can cross that bridge when we come to it.

mmcloughlin commented 10 months ago

Oh. I think I was thrown off by the fact that the PCALIGN instruction has existed in the assembler for a long time. But as you've said there wasn't backend support for it in amd64 until recently golang/go#56474.

PR #420 adds it but is failing assembler tests on Go 1.20 and 1.21 for this reason.

This is another use case for avo tracking the Go version required for each instruction #84.

egonelbre commented 10 months ago

You can use go1.22rc1 for testing that. As for the implementation, I don't have that much experience with avo to have an opinion on how to implement it.

For now I did this in my own code:

Instruction(&ir.Instruction{
    Opcode:   "PCALIGN",
    Operands: []Op{Imm(1024)},
})
mmcloughlin commented 10 months ago

You can use go1.22rc1 for testing that.

For sure. The problem is the test I have that validates the avo instruction database by assembling a generated file with one example of every instruction form. This test runs in CI with the installed Go version, so at the moment with Go 1.20 and 1.21. Therefore the tests fail when I add the PCALIGN instruction.

The fix would be to either install a pinned Go version for the purpose of that test. Or alternatively, implement some form of #84 and store the version required for each instruction, which would then allow me to limit which instructions appear in the assembler test file.