fix various bugs in the x86 processor spec#9097
Open
jcrussell wants to merge 2 commits intoNationalSecurityAgency:masterfrom
Open
fix various bugs in the x86 processor spec#9097jcrussell wants to merge 2 commits intoNationalSecurityAgency:masterfrom
jcrussell wants to merge 2 commits intoNationalSecurityAgency:masterfrom
Conversation
Specifically: * ARPL uses CF instead of ZF for conditional RPL adjustment * DAA/DAS incorrect CF computation and missing result flags * SHR by-1 OF flag unconditionally set to 0 * RDTSCP incorrectly restricted to 64-bit mode only * SYSEXIT uses wrong register mapping * FCMOVNBE condition logic error * FYL2X/FYL2XP1 missing log2 computation * CVTPD2DQ uses trunc() instead of round() * SUBB MMX missing byte lane [8,8] * PSLLD/PSLLQ XMM use per-element shift count * PCMPxSTRx missing implicit EAX/EDX inputs and EFLAGS outputs * CET WRSSQ/WRUSSQ duplicate 0x0F byte in encoding * FMA VEX_W0 missing on PS YMM 231-form constructors * VTESTPS YMM sign bit offset errors * AVX-512 VANDPS YMM dead variable * AVX-512 VMOVHLPS copies src2 to both halves * AVX-512 VMOVHPD load form dead variable * VPSUBSW ZMM form multiple errors * CVTPD2PI uses trunc() instead of round() * PSRAD XMM uses full 128-bit shift count * VMULPS YMM missing 8th element * FPREM/FPREM1 broken remainder computation * FDECSTP clears wrong condition code bit * VMCLEAR incorrectly restricted to 64-bit mode * SYSCALL does not model RCX and R11 clobber * VRANGESS missing imm8 operand * VFPCLASSSD/VFPCLASSSS missing imm8 operand * AVX-512 VUCOMISD/VUCOMISS missing EFLAGS computation * MASKMOVDQU incorrectly assigns to XmmReg1 * DAS has same CF computation and missing flags issues as DAA * XGETBV/XSETBV ignore ECX selector
ryanmkurtz
added
Feature: Processor/x86
Status: Triage
GhidorahRex
reviewed
Apr 6, 2026
| @@ -10248,20 +10273,20 @@ define pcodeop crc32; | |||
| @endif | |||
|
|
|||
| define pcodeop pcmpestri; | |||
| :PCMPESTRI XmmReg, m128, imm8 is vexMode=0 & $(PRE_66) & byte=0x0F; byte=0x3A; byte=0x61; XmmReg ... & m128; imm8 { ECX = pcmpestri(XmmReg, m128, imm8:8); } | |||
| :PCMPESTRI XmmReg1, XmmReg2, imm8 is vexMode=0 & $(PRE_66) & byte=0x0F; byte=0x3A; byte=0x61; xmmmod=3 & XmmReg1 & XmmReg2; imm8 { ECX = pcmpestri(XmmReg1, XmmReg2, imm8:8); } | |||
| :PCMPESTRI XmmReg, m128, imm8 is vexMode=0 & $(PRE_66) & byte=0x0F; byte=0x3A; byte=0x61; XmmReg ... & m128; imm8 { ECX = pcmpestri(XmmReg, m128, imm8:8, EAX, EDX); CF = (ECX != 0); ZF = (ECX != 0); SF = (ECX != 0); OF = (ECX != 0); } | |||
Collaborator
There was a problem hiding this comment.
These flags don't look like they're set correctly. According to the manual ZF, for example, is set if abs(EDX) < 16. ECX seems to only be set if IntRes2 is 0, so it has no correlation to the value of EDX, same with SF. Same with the other PCMP* instructions.
There's an assumption in the original pcodeop that the flags would be updated internally. If that's no longer true, then ECX needs to be updated according to the value of IntRes2 - if the result is 0, ECX should be set to 16.
GhidorahRex
reviewed
Apr 6, 2026
| @@ -4670,23 +4674,23 @@ define pcodeop smm_restore_state; | |||
| shrdflags(tmp,rm64,count); shiftresultflags(rm64,count); } | |||
| @endif | |||
|
|
|||
| :SHR rm8,n1 is vexMode=0 & byte=0xD0; rm8 & n1 & reg_opcode=5 ... { CF = rm8 & 1; OF = 0; rm8 = rm8 >> 1; resultflags(rm8); } | |||
| :SHR rm8,n1 is vexMode=0 & byte=0xD0; rm8 & n1 & reg_opcode=5 ... { OF = (rm8 >> 7) & 1; CF = rm8 & 1; rm8 = rm8 >> 1; resultflags(rm8); } | |||
Collaborator
There was a problem hiding this comment.
Is there a specific reason you used (rm8 >> 7) & 1 as opposed to rm8 s< 0 here?
GhidorahRex
reviewed
Apr 6, 2026
| @@ -6724,31 +6733,39 @@ CMPSS_OPERAND: ", "^imm8 is imm8 { } | |||
| :CVTPD2DQ XmmReg, m128 is vexMode=0 & $(PRE_F2) & byte=0x0F; byte=0xE6; m128 & XmmReg ... | |||
| { | |||
| local m:16 = m128; | |||
| XmmReg[0,32] = trunc( m[0,64] ); | |||
| XmmReg[32,32] = trunc( m[64,64] ); | |||
| local tmp0:8 = round( m[0,64] ); | |||
Collaborator
There was a problem hiding this comment.
I believe trunc is correct here. CVTPD2DQ converts a floating point number into an integer, rounding towards zero. round rounds a floating point number to the nearest integer in floating point format, so that's almost surely not what we want to do here.
GhidorahRex
reviewed
Apr 6, 2026
| @@ -7146,7 +7163,7 @@ define pcodeop lddqu; | |||
| :LDDQU XmmReg, m128 is vexMode=0 & $(PRE_F2) & byte=0x0F; byte=0xF0; XmmReg ... & m128 { XmmReg = lddqu(XmmReg, m128); } | |||
|
|
|||
| define pcodeop maskmovdqu; | |||
| :MASKMOVDQU XmmReg1, XmmReg2 is vexMode=0 & $(PRE_66) & byte=0x0F; byte=0xF7; XmmReg1 & XmmReg2 { XmmReg1 = maskmovdqu(XmmReg1, XmmReg2); } | |||
| :MASKMOVDQU XmmReg1, XmmReg2 is vexMode=0 & $(PRE_66) & byte=0x0F; byte=0xF7; XmmReg1 & XmmReg2 { maskmovdqu(XmmReg1, XmmReg2); } | |||
Collaborator
There was a problem hiding this comment.
maskmovq should probably be updated at the same time.
Collaborator
|
Overall, a lot of really great fixes! |
Fixes issues raised by @GhidorahRex. Adds new bug fx for MASKMOVQ.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Specifically: