Bug 510864 - Add SSE4.1 PMAXSD and PMINSD instructions support for 32-bit x86
Summary: Add SSE4.1 PMAXSD and PMINSD instructions support for 32-bit x86
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (other bugs)
Version First Reported In: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Alexandra Hajkova
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2025-10-21 11:51 UTC by Alexandra Hajkova
Modified: 2025-12-02 16:15 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed/Implemented In:
Sentry Crash Report:


Attachments
patch (20.48 KB, patch)
2025-11-04 19:10 UTC, Alexandra Hajkova
Details
patch (5.15 KB, patch)
2025-11-11 12:40 UTC, Alexandra Hajkova
Details
patch (5.32 KB, patch)
2025-11-11 12:40 UTC, Alexandra Hajkova
Details
patch (6.93 KB, patch)
2025-11-26 17:01 UTC, Alexandra Hajkova
Details
patch (9.13 KB, patch)
2025-11-26 17:06 UTC, Alexandra Hajkova
Details

Note You need to log in before you can comment on or make changes to this bug.
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.