-
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
proposal: cmd/asm: untyped imm8 operand class for x86 asm #21528
Comments
I want to emphasize: it does not break any existing assembly code; |
Change https://golang.org/cl/57570 mentions this issue: |
I tried pretty hard in the instruction tables to distinguish the ones that interpret the argument as signed vs unsigned. If one is wrong I'd like to fix that. Do you have an example of one that's wrong? If VPBLENDD only cares about bits (and 0x80 is a bit and not the sign), then it should be defined to take uimm8. Your comment:
makes me think that VPBLENDD is already correct, or else it wouldn't assemble. |
@rsc, When instructions are generated from, for example, As a side effect of this, the result behavior is also more compatible with widely used assemblers. |
To make it clear, current 4 operand instructions semantics for immediate argument are the same as |
I don't want to be sloppy here. Assembly is hard enough to get right. I want to be precise. Either it's a signed value or an unsigned value, and it's OK for the assembler to insist that you know which one. I understand that other assemblers take a different approach, and that's fine, but it's not our approach. Our approach is to be picky, to try to help avoid bugs. |
@rsc, I see. Maybe we can make x86spec format express signed/unsigned immediate arguments? |
@rsc, I may be wrong, but it seems like var yshl = []ytab{
{Yi1, Ynone, Yml, Zo_m, 2},
{Yi32, Ynone, Yml, Zibo_m, 2}, // <- here
{Ycl, Ynone, Yml, Zo_m, 2},
{Ycx, Ynone, Yml, Zo_m, 2},
} If I can check other instructions one-by-one later. |
x86.csv does have this information: it says imm8 for signed or imm8u for unsigned. (Sorry for the late reply.) |
The |
Change https://golang.org/cl/62190 mentions this issue: |
Change "yshl" and "yshb" immediate oclass from Yi32 to Yu8. This forbids: - negative shift counts - shift counts that not fit into 8bit Affects: RCL{B,L,Q,W} RCR{B,L,Q,W} ROL{B,L,Q,W} ROR{B,L,Q,W} SAL{B,L,Q,W} SAR{B,L,Q,W} SHL{B,L,Q,W} SHR{B,L,Q,W} Issue #21528 has some additional context about this change. Change-Id: I60884cb2b41a860820889fcd878ca6f564006b4a Reviewed-on: https://go-review.googlesource.com/62190 Run-TryBot: Iskander Sharipov <iskander.sharipov@intel.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Russ Cox <rsc@golang.org>
Going to close this. |
Many amd64 instructions take immediate operand without auto sign extension.
When we only care about binary representation, both "$128" and "$-128" have the same value.
In Intel manual,
VPBLENDD
haveimm8
operand class. This means "signed 8bit value".But semantics only care about bits, so it totally makes sense to use
0xF0
as an argument.Well, src/crypto/sha512/sha512block_amd64.s:524 does exactly this:
VPBLENDD $0xF0, Y2, Y4, Y4
GAS
also happily accepts both positive and negative immediate values.I propose backwards-compatible change that includes a new opclass -
Yimmb
which is a union of {Yi0, Yi1, Yi8, Yu8, Yu7} opclasses viaycover
.From a user point of view, this change will make it possible to express immediate argument in both signed and unsigned form.
This first step helps us to reduce
yvex_xyi3
to 2 ytab entries.The other step that may be proposed later: allowing more instructions to accept
Yimmb
opclass instead ofYi8
/Yu8
.pros:
yvex_xyi3
)Even if
asm6.go
is going to be automatically generated somewhere in future,the exact dates are unknown, so this simple change may make sense for the current.
Implementation:
Possible patch is provided below.
Yimmb.patch
The text was updated successfully, but these errors were encountered: