-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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/asm: confusing names, encodings for VEX instructions #14068
Comments
When I submitted original CL (https://golang.org/cl/16434), |
@TocarIP thanks for the comments. I agree about HD following the
established pattern for a general move; it's really the follow-on effects
on the other VEX instructions that are the problem. Even so I think I will
rename the HD move too. If we want a generic 16-byte move alias I am
inclined to just call them MOV16U and MOV16A. I've been meaning to make
common aliases MOV1, MOV2, MOV4, MOV8 across all the systems anyway (right
now there are inconsistencies, like MOVW means 4 bytes on ARM but 2 bytes
on x86). But I'll leave that for Go 1.7.
In general I am leaning toward making the Go assembly match GNU assembler
as much as possible in the SSE, AVX, and other extensions, for exactly the
reasons you mentioned. So I'll add the Y registers now, make both forms of
the VEX instructions available, and queue further cleanup for Go 1.7. (I'm
going to leave the integer instructions alone, we'll keep having AX instead
of AX/EAX/RAX and so on.)
Thanks again.
|
I would agree that adding the Y register names seems like the right thing to do. I did not realize at the time that the V* opcodes had both xmm and ymm forms. |
CL https://golang.org/cl/18846 mentions this issue. |
CL https://golang.org/cl/18850 mentions this issue. |
CL https://golang.org/cl/18849 mentions this issue. |
CL https://golang.org/cl/18852 mentions this issue. |
Not recognized in any instructions yet, but this lets the assembler parse them at least. For #14068. Change-Id: Id4f7329a969b747a867ce261b20165fab2cdcab8 Reviewed-on: https://go-review.googlesource.com/18846 Reviewed-by: Rob Pike <r@golang.org>
Tests for this and many other instructions are in a separate followup CL. For #14068. Change-Id: I6955315996a34d7fb79369b9d9a0119d11745e85 Reviewed-on: https://go-review.googlesource.com/18849 Reviewed-by: Rob Pike <r@golang.org>
Generated by x86test, from https://golang.org/cl/18842 (still in progress). The commented out lines are either missing or misspelled or incorrectly handled instructions. For #4816, #8037, #13822, #14068, #14069. Change-Id: If309310c97d9d2a3c71fc64c51d4a957e9076ab7 Reviewed-on: https://go-review.googlesource.com/18850 Reviewed-by: Rob Pike <r@golang.org> Run-TryBot: Russ Cox <rsc@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org>
I have constructed a fairly exhaustive test suite for the x86 assembler and identified some problems. The ones in this issue were added during the Go 1.6 cycle and should be corrected before the release. I intend to do this and also check in the tests.
Added in 9dd81d6, https://golang.org/cl/14127, Oct 5 2015:
It took me a while to figure out that VMOVDQU, VMOVDQA, and VMOVNTDQ have been renamed to MOVHDU, MOVHDA, and MOVNTHD. That's very strange. We don't use HD in place of DQ anywhere else, nor do we drop the leading V. We do use "O" in place of "DQ" in a number of places, and that would have been an OK convention to follow, but we also install aliases for the Intel names because this is all so confusing, and that wasn't done here, making the spellings chosen in this CL basically impossible to find. I only found them because I was looking for CLs related to the others below that caused test failures in instructions with more predictable names. These should be renamed, and I will do that.
Once renamed, there is still the issue, not mentioned anywhere in the CL description nor the code that the X registers (128-bit) are being reused here as identifiers for Y registers (256-bit that hold the 128-bit forms in their bottom halves). The motivation for changing VMOVDQ to MOVHD may have been to emphasize this fact, which I appreciate, but it would have been nice to discuss the overloading explicitly. I am not sure it's a great idea, since it leads to very confusing assembly. At the very least that fact needs clear documentation. It was just slipped in as far as I can tell.
The overloading of X and Y registers is handled here by changing the name but then all the followup instructions just silently used Y registers without name changes. In many cases it's hard to tell the difference, assuming you're okay with asking for an X instruction and getting one that does what you asked but also sets the high Y bits (the VEX instructions operating on X registers are documented to zero the high Y bits, not use them). But in at least one case it matters greatly: VPTEST on X registers is defined to only consider the low 128 bits in the comparison. If it silently compares the whole 256-bit Y register instead, that's sure to surprise someone (along with the fact that the top half has not been zeroed by all the earlier V instructions).
This is going to take a while to figure out, clean up, and document. Good thing there are a few days left before the release candidate.
Added in 0e23ca4, https://golang.org/cl/16434, Nov 2 2015:
Added in 967564b, https://golang.org/cl/16481, Nov 5 2015:
Added in 321a407, https://golang.org/cl/16484, Nov 6 2015:
Added in b597e1e, https://golang.org/cl/16773, Nov 24 2015:
Added in 1d1f2fb, https://golang.org/cl/18593, Jan 13 2016:
The problem here, confusing both objdump and xed, is that there are two REX bytes! The 44 48 should be 4C. I'm amazed this executes. Maybe it doesn't.
The text was updated successfully, but these errors were encountered: