Bug 510864

Summary: Add SSE4.1 PMAXSD and PMINSD instructions support for 32-bit x86
Product: [Developer tools] valgrind Reporter: Alexandra Hajkova <ahajkova>
Component: generalAssignee: Alexandra Hajkova <ahajkova>
Status: RESOLVED FIXED    
Severity: normal CC: mark
Priority: NOR    
Version First Reported In: unspecified   
Target Milestone: ---   
Platform: Other   
OS: Linux   
Latest Commit: Version Fixed/Implemented In:
Sentry Crash Report:
Attachments: patch
patch
patch
patch
patch

Description Alexandra Hajkova 2025-10-21 11:51:54 UTC
include <stdio.h>

int main()
{
    int t;
    int one[1] = {1};
    asm volatile(
            "pxor %%xmm0,%%xmm0\n\t"
            "movd %1,%%xmm1\n\t"
            "pmaxsd %%xmm1,%%xmm0\n\t"
            "movd %%xmm0,%0\n\t"
            : "=r"(t) : "m"(one));
    printf(" %d\n", t);
    return 0;
}

It'll work fine on 64-bit and if you compile it on 32-bit it will work fine but Valgrind will complain about illegal instruction
Comment 1 Mark Wielaard 2025-10-21 12:08:13 UTC
most of SSE4.1 should be implemented for amd64, but not for x86
none/tests/amd64/sse4-64.c has a lot more test cases than none/tests/x86/sse4-x86.c
so that might be a good way to start (add some tests from amd64 to x86 and then lift the implementation).
Comment 2 Alexandra Hajkova 2025-11-04 19:10:40 UTC
Created attachment 186499 [details]
patch

Let's refactor the current code a bit to make it easier to use existing amd64 code for x86 tests.
Comment 3 Alexandra Hajkova 2025-11-11 12:40:20 UTC
Created attachment 186695 [details]
patch

sse4-common.h: make changes to make test_PMAXSD usable for 32bit
    
Modify DO_imm_r_r macro called by test_PMAXSD to use xmm7 register
when testing on 32bit.
Comment 4 Alexandra Hajkova 2025-11-11 12:40:57 UTC
Created attachment 186696 [details]
patch

 Add pmaxsd support for x86 32 bit
    
Support pmaxsd instruction in guest_x86_toIR.c and host_x86_isel.c
and modify the none/tests/x86/sse4-x86.stdout.exp to match pmaxsd
support.
Comment 5 Mark Wielaard 2025-11-19 17:54:23 UTC
(In reply to Alexandra Hajkova from comment #4)
> Created attachment 186696 [details]
> patch
> 
>  Add pmaxsd support for x86 32 bit
>     
> Support pmaxsd instruction in guest_x86_toIR.c and host_x86_isel.c
> and modify the none/tests/x86/sse4-x86.stdout.exp to match pmaxsd
> support.

This looks very nice. I would also handle PMINSD in the guest decoder and add test_PMINSD, since you already have it in the instruction selector.

Nitpick, some extra whitespace in VEX/priv/guest_amd64_toIR.c

The patch buildup is nice too.  First pulling out the common code. Then making it usable for building on 32bit. Then implementing the instructions and testing them.
Comment 6 Alexandra Hajkova 2025-11-26 17:01:52 UTC
Created attachment 187193 [details]
patch
Comment 7 Alexandra Hajkova 2025-11-26 17:06:35 UTC
Created attachment 187194 [details]
patch
Comment 8 Mark Wielaard 2025-11-26 17:34:40 UTC
These three patches look good. Just needs a NEWS entry. Please apply.