Page MenuHomePhabricator

Convert __m64 intrinsics to unconditionally use SSE2 instead of MMX instructions.
Needs ReviewPublic

Authored by jyknight on Aug 30 2020, 3:29 PM.



Preliminary patch, posted to go along with discussion on llvm-dev.

3DNow! intrinsics are not converted, as of yet.

Tests have not been updated to match new expected IR output. Currently failing:
Clang :: CodeGen/attr-target-x86-mmx.c
Clang :: CodeGen/mmx-builtins.c
Clang :: CodeGen/mmx-shift-with-immediate.c
Clang :: Headers/xmmintrin.c

Diff Detail

Unit TestsFailed

470 mslinux > Clang.CodeGen::attr-target-x86-mmx.c
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -cc1 -internal-isystem /mnt/disks/ssd0/agent/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -triple i386-linux-gnu -emit-llvm /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/attr-target-x86-mmx.c -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/attr-target-x86-mmx.c
1,650 mslinux > Clang.CodeGen::mmx-builtins.c
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -cc1 -internal-isystem /mnt/disks/ssd0/agent/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc -flax-vector-conversions=none -ffreestanding /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/mmx-builtins.c -triple=x86_64-apple-darwin -target-feature +ssse3 -emit-llvm -o - -Wall -Werror | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/mmx-builtins.c
80 mslinux > Clang.CodeGen::mmx-shift-with-immediate.c
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -mmmx -target i386-unknown-unknown -emit-llvm -S /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/mmx-shift-with-immediate.c -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/CodeGen/mmx-shift-with-immediate.c
100 mslinux > Clang.Headers::xmmintrin.c
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/build/bin/clang -cc1 -internal-isystem /mnt/disks/ssd0/agent/llvm-project/build/lib/clang/12.0.0/include -nostdsysteminc /mnt/disks/ssd0/agent/llvm-project/clang/test/Headers/xmmintrin.c -ffreestanding -triple x86_64-apple-macosx10.9.0 -emit-llvm -o - | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/clang/test/Headers/xmmintrin.c
970 mswindows > Clang.CodeGen::attr-target-x86-mmx.c
Script: -- : 'RUN: at line 1'; c:\ws\w16-1\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w16-1\llvm-project\premerge-checks\build\lib\clang\12.0.0\include -nostdsysteminc -triple i386-linux-gnu -emit-llvm C:\ws\w16-1\llvm-project\premerge-checks\clang\test\CodeGen\attr-target-x86-mmx.c -o - | c:\ws\w16-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16-1\llvm-project\premerge-checks\clang\test\CodeGen\attr-target-x86-mmx.c
View Full Test Results (8 Failed)

Event Timeline

jyknight created this revision.Aug 30 2020, 3:29 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2020, 3:29 PM
jyknight requested review of this revision.Aug 30 2020, 3:29 PM
craig.topper added inline comments.Aug 30 2020, 10:44 PM

I think you we should use __v8qu to match what we do in emmintrin.h. We don't currently set nsw on signed vector arithmetic, but we should be careful in case that changes in the future.


I think we probably want to use a v2su or v2si here. Using v1di scalarizes and splits on 32-bit targets. On 64-bit targets it emits GPR code.


Need to use v8qs here to force "signed char" elements. v8qi uses "char" which has platform dependent signedness or can be changed with a command line.


Same here


Is this needed?


I don't think this change is needed. And I think the operands are in the wrong order.


I'm worried that using v1di with the shuffles will lead to scalarization in the type legalizer. Should we use v2si instead?


This doesn't guarantee zeroes in bits 15:8 does it?


Does this work with large pages?