Page MenuHomePhabricator

[X86] Fix tile register spill issue.
ClosedPublic

Authored by LuoYuanke on Dec 29 2020, 4:08 AM.

Details

Summary

The tile register spill need 2 instructions.
%46:gr64_nosp = MOV64ri 64
TILESTORED %stack.2, 1, killed %46:gr64_nosp, 0, $noreg, %43:tile
The first instruction load the stride to a GPR, and the second
instruction store tile register to stack slot. The optimization of merge
spill instruction is done after register allocation. And spill tile
register need create a new virtual register to for stride, so we can't
hoist tile spill instruction in postOptimization() of register
allocation. We can't hoist TILESTORED alone and we can't hoist the 2
instuctions together because MOV64ri will clobber some GPR. This patch
is to disble the spill merge for any spill which need 2 instructions.

Diff Detail

Unit TestsFailed

TimeTest
30 msx64 debian > LLVM.CodeGen/X86/AMX::amx-spill-merge.ll
Script: -- : 'RUN: at line 2'; /mnt/disks/ssd0/agent/llvm-project/build/bin/llc < /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/X86/AMX/amx-spill-merge.ll -mtriple=x86_64-unknown-unknown -mattr=+amx-int8 -mattr=+avx512f -verify-machineinstrs | /mnt/disks/ssd0/agent/llvm-project/build/bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/llvm/test/CodeGen/X86/AMX/amx-spill-merge.ll
2,190 msx64 debian > libarcher.races::lock-unrelated.c
Script: -- : 'RUN: at line 13'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang -fopenmp -pthread -fno-experimental-isel -g -O1 -fsanitize=thread -I /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests -I /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/runtime/src -L /mnt/disks/ssd0/agent/llvm-project/build/lib -Wl,-rpath,/mnt/disks/ssd0/agent/llvm-project/build/lib /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c -o /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp -latomic && env TSAN_OPTIONS='ignore_noninstrumented_modules=0:ignore_noninstrumented_modules=1' /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/deflake.bash /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp 2>&1 | tee /mnt/disks/ssd0/agent/llvm-project/build/projects/openmp/tools/archer/tests/races/Output/lock-unrelated.c.tmp.log | /mnt/disks/ssd0/agent/llvm-project/build/./bin/FileCheck /mnt/disks/ssd0/agent/llvm-project/openmp/tools/archer/tests/races/lock-unrelated.c
50 msx64 windows > LLVM.CodeGen/X86/AMX::amx-spill-merge.ll
Script: -- : 'RUN: at line 2'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\llc.exe < C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\X86\AMX\amx-spill-merge.ll -mtriple=x86_64-unknown-unknown -mattr=+amx-int8 -mattr=+avx512f -verify-machineinstrs | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\X86\AMX\amx-spill-merge.ll

Event Timeline

LuoYuanke created this revision.Dec 29 2020, 4:08 AM
LuoYuanke requested review of this revision.Dec 29 2020, 4:08 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 29 2020, 4:08 AM
wxiao3 added inline comments.Jan 3 2021, 7:20 PM
llvm/lib/CodeGen/InlineSpiller.cpp
272

The function name is misleading since the function just get intervals for definition registers.
Suggest using more specific name such as "getVDefInterval" or something else.

BTW, I guess there may be existing code can provide similar functionality of creating intervals for new registers.
@qcolombet may provide more information.

LuoYuanke updated this revision to Diff 314313.Jan 3 2021, 8:32 PM

Address Wei's comments.

LuoYuanke added inline comments.Jan 3 2021, 8:36 PM
llvm/lib/CodeGen/InlineSpiller.cpp
272

The proposed name looks good to me. Renamed the function to "getVDefInterval".

wxiao3 accepted this revision.Jan 6 2021, 6:37 PM

LGTM. But I suggest you waiting for one or two days to see if @qcolombet or others object.

This revision is now accepted and ready to land.Jan 6 2021, 6:37 PM

LGTM. But I suggest you waiting for one or two days to see if @qcolombet or others object.

Thank Wei for the review. Sure, I'll wait for a few days to see if there is any object from others.

This revision was automatically updated to reflect the committed changes.