Page MenuHomePhabricator

Fix X86-64 ABIT issue when splitting an i128 into two i64 during function call
Needs RevisionPublic

Authored by serge-sans-paille on Aug 12 2021, 8:09 AM.

Details

Summary

X86-64 ABI mandates that wrt. argument passing, i128 is considered as a struct {i64, i64}
and thus that it cannot be split across register and stack.

GCC already implements this behavior, while clang doesn't, which can cause
subtle runtime issues when mixing both object files.

As a side effect, share some code between 32 and 64 bit implementation.

Diff Detail

Unit TestsFailed

TimeTest
1,140 msx64 debian > Builtins-x86_64-linux.Builtins-x86_64-linux::divmodti4_test.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -gline-tables-only -m64 -fno-builtin -I /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/builtins -nodefaultlibs /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/builtins/Unit/divmodti4_test.c /var/lib/buildkite-agent/builds/llvm-project/build/./lib/clang/14.0.0/lib/linux/libclang_rt.builtins-x86_64.a -lc -lm -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/divmodti4_test.c.tmp && /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/divmodti4_test.c.tmp
4,620 msx64 debian > Builtins-x86_64-linux.Builtins-x86_64-linux::udivmodti4_test.c
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -gline-tables-only -m64 -fno-builtin -I /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/lib/builtins -nodefaultlibs /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/builtins/Unit/udivmodti4_test.c /var/lib/buildkite-agent/builds/llvm-project/build/./lib/clang/14.0.0/lib/linux/libclang_rt.builtins-x86_64.a -lc -lm -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/udivmodti4_test.c.tmp && /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/builtins/Unit/X86_64LinuxConfig/Output/udivmodti4_test.c.tmp
290 msx64 windows > Clang Tools.clang-tidy/checkers::readability-container-data-pointer.cpp
Script: -- : 'RUN: at line 1'; C:/Python39/python.exe C:/ws/w3/llvm-project/premerge-checks/clang-tools-extra/test/../test\clang-tidy\check_clang_tidy.py C:\ws\w3\llvm-project\premerge-checks\clang-tools-extra\test\clang-tidy\checkers\readability-container-data-pointer.cpp readability-container-data-pointer C:\ws\w3\llvm-project\premerge-checks\build\tools\clang\tools\extra\test\clang-tidy\checkers\Output\readability-container-data-pointer.cpp.tmp

Event Timeline

serge-sans-paille requested review of this revision.Aug 12 2021, 8:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 12 2021, 8:09 AM

This diff relies on the ArgumentNeedsConsecutiveRegisters property which is unused for X86. I'm new to codegen and will happily follow another track.

pengfei requested changes to this revision.Sep 14 2021, 6:48 PM
pengfei added inline comments.
llvm/lib/Target/X86/X86CallingConv.td
545

This is not correct. See the generated code in large-argument-count-i128.ll:12. We want the i128 (consecutive 2 i64) memory is 16 byte aligned instead of 2 i64 that aligned 16 bytes.

llvm/test/CodeGen/X86/large-argument-count-i128.ll
12

This should be 8?

This revision now requires changes to proceed.Sep 14 2021, 6:48 PM