New issue
Advanced search Search tips
Starred by 5 users

Issue metadata

Status: Accepted
Owner:
Cc:

Blocking:
issue chromium:931926



Sign in to add a comment
link

Issue 22: Fix support for the Microsoft x64 calling convention in Subzero

Reported by nicolasc...@google.com, Oct 28 2016 Project Member

Issue description

Subzero currently assumes the System V AMD64 ABI. We need to also support Microsoft's x86-64 calling convention. https://msdn.microsoft.com/en-us/library/ms235286.aspx
 

Comment 1 by bugdroid1@chromium.org, Oct 28 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/pnacl-subzero/+/73ae4fdc32345ad1dc44cc5268204174555b2b55

commit 73ae4fdc32345ad1dc44cc5268204174555b2b55
Author: Nicolas Capens <capn@google.com>
Date: Fri Oct 28 15:24:50 2016

Preserve rsi and rdi when using Microsoft x86-64 calling convention.

Also, their priority is lowered so that registers which are scratch on both
Unix and Windows are preferred by the register allocator.

BUG=swiftshader:22

Change-Id: Id55d8c8b8c106947e3041a082099069d7c6c6ed0
Reviewed-on: https://chromium-review.googlesource.com/404503
Reviewed-by: Jim Stichnoth <stichnot@chromium.org>
Tested-by: Jim Stichnoth <stichnot@chromium.org>

[modify] https://crrev.com/73ae4fdc32345ad1dc44cc5268204174555b2b55/src/IceInstX8664.def
[modify] https://crrev.com/73ae4fdc32345ad1dc44cc5268204174555b2b55/src/IceTargetLoweringX8664Traits.h

Comment 2 by bugdroid1@chromium.org, Jan 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/native_client/src/native_client.git/+/01d3ae00d277726b8aaee572120a5ba2a31d511e

commit 01d3ae00d277726b8aaee572120a5ba2a31d511e
Author: Jim Stichnoth <stichnot@chromium.org>
Date: Wed Jan 04 07:04:41 2017

PNaCl: Update subzero revision in pnacl/COMPONENT_REVISIONS

This pulls in the following Subzero changes:

ec92917: (jaydeep.patil@imgtec.com) [SubZero] Legalize load, store for MIPS post lower
4683237: (sagar.thakur@imgtec.com) [Subzero][MIPS32] Fix alloca alignment and offset for Om1 and O2 optimization
45e4d5e: (jaydeep.patil@imgtec.com) [SubZero] Handle relocatable constants for MIPS
3a01f33: (jaydeep.patil@imgtec.com) [SubZero] Implement Fcmp, ICmp, Cast and Select for vector type
f52cea4: (capn@google.com) Fix unpacking from a single vector.
7638e27: (capn@google.com) Add x86 vector packing instructions.
0e90622: (capn@google.com) Generate error on unexpected intrisics.
89be887: (sagar.thakur@imgtec.com) [Subzero][MIPS32] Account for variable alloca alignment bytes in addProlog
1448d95: (capn@google.com) Optimize shuffles corresponding to x86 punpckh instructions.
7145e69: (stichnot@chromium.org) Subzero: Fix compiler warnings.
32f9cce: (capn@google.com) Fix 64-bit pointer type for non-x32 ABIs.
ef8210d: (capn@google.com) Implement vector packing intrinsics.
8b8af82: (capn@google.com) Implement bitcast between i32 and (emulated) v4i8.
e3cabda: (capn@google.com) Implement vector sign mask intrinsic.
d0e3030: (capn@google.com) Assert that PNaCl bitcode only uses 128-bit vector casts.
c9e91af: (capn@google.com) Remove verified asserts.
61593fb: (capn@google.com) Fix unit tests.
a3688ea: (capn@google.com) Fix two-vector unpack case.
a7979bf: (jaydeep.patil@imgtec.com) [SubZero] Fix f64 to/from i64 moves
0dabe18: (makdstefan@gmail.com) Subzero, MIPS32: Remove --skip-unimplemented from lit tests
130aca7: (jaydeep.patil@imgtec.com) [SubZero] Generate relocations for MIPS
73ae4fd: (capn@google.com) Preserve rsi and rdi when using Microsoft x86-64 calling convention.
7ad028e: (makdstefan@gmail.com) This patch enables running a couple more of lit tests for MIPS32
956cfd6: (capn@google.com) Generalize the Sqrt intrinsic to process vectors.
13cde0f: (capn@google.com) Implement integer vector multiply intrinsics.
67a49b5: (capn@google.com) Implement saturated vector add/subtract.
0c4c07d: (jaydeep.patil@imgtec.com) [SubZero] Fix code generation for vector type
f8c9977: (makdstefan@gmail.com) Subzero, MIPS32: Stacksave/Stackrestore implementation
f0d12c3: (capn@google.com) Implement floating-point rounding intrinsic.
3da9f65: (jaydeep.patil@imgtec.com) [SubZero] Generate MIPS.abiflags section
21f78bb: (jaydeep.patil@imgtec.com) [SubZero] Utilize instructions with immediate operands
83425de: (capn@google.com) Support 64-bit jump tables with LP64 data model.
6e03343: (makdstefan@gmail.com) Subzero, MIPS32: Sandbox initial patch
becb85f: (sagar.thakur@imgtec.com) [Subzero][MIPS] Implements atomic intrinsics for MIPS32
8208e75: (makdstefan@gmail.com) Subzero, MIPS32: Changes for improving sandbox crosstest results
2220990: (capn@google.com) Fix offset adjustment in x86 address optimization.
ef18fc5: (capn@google.com) Match sub-vector load/store operand order to regular load/store.
8be6975: (jaydeep.patil@imgtec.com) [SubZero] Fix size of arguments on stack
e1e1783: (capn@google.com) Fix skipping deleted instructions before replacing operands.
a29da90: (capn@google.com) Ensure that the sub-vector load destination is a register.
579b1b3: (capn@google.com) Generalize vector shuffling to accept any operand.
373913f: (stichnot@chromium.org) Subzero: Legalize the movzx argument.
35bbca3: (stichnot@chromium.org) Subzero: Fix multiply defined symbols in Windows/g++ build.

BUG=  swiftshader:24 
BUG=  swiftshader:9 
BUG= swiftshader:22
BUG= https://bugs.chromium.org/p/nativeclient/issues/detail?id=4384
BUG=  swiftshader:15 
TEST= PNaCl toolchain trybots
R=dschuff@chromium.org

Review-Url: https://codereview.chromium.org/2602613002 .

[modify] https://crrev.com/01d3ae00d277726b8aaee572120a5ba2a31d511e/pnacl/COMPONENT_REVISIONS

Comment 3 by nicolasc...@google.com, Jan 13 2017

Project Member
Status: Fixed (was: Accepted)

Comment 4 by bugdroid1@chromium.org, May 3 2017

The following revision refers to this bug:
  https://swiftshader.googlesource.com/SwiftShader.git/+/73ae4fdc32345ad1dc44cc5268204174555b2b55

commit 73ae4fdc32345ad1dc44cc5268204174555b2b55
Author: Nicolas Capens <capn@google.com>
Date: Fri Oct 28 19:05:28 2016

Preserve rsi and rdi when using Microsoft x86-64 calling convention.

Also, their priority is lowered so that registers which are scratch on both
Unix and Windows are preferred by the register allocator.

BUG=swiftshader:22

Change-Id: Id55d8c8b8c106947e3041a082099069d7c6c6ed0
Reviewed-on: https://chromium-review.googlesource.com/404503
Reviewed-by: Jim Stichnoth <stichnot@chromium.org>
Tested-by: Jim Stichnoth <stichnot@chromium.org>

[modify] https://crrev.com/73ae4fdc32345ad1dc44cc5268204174555b2b55/src/IceInstX8664.def
[modify] https://crrev.com/73ae4fdc32345ad1dc44cc5268204174555b2b55/src/IceTargetLoweringX8664Traits.h

Comment 5 by capn@chromium.org, Feb 20 (3 days ago)

Project Member
Cc: senorblanco@chromium.org
Status: Accepted (was: Fixed)
Reopening as we discovered in Issue chromium:931926 that the Microsoft x86-64 calling convention isn't fully supported yet by Subzero. Specifically XMM6-15 should be callee-saved.

https://swiftshader-review.googlesource.com/c/SwiftShader/+/24908 currently simply changes them from being designated as scratch registers to reserved, but TargetX86Base<TraitsType>::addProlog() assumes all callee-saved registers to be general purpose integer ones (GPR) so it emits push instructions using _push_reg().

Fixing this requires ensuring that we have sufficient 16-byte stack slots for preserving/restoring these XMM registers, most preferably 16-byte aligned, emitting movaps store/load instructions instaed of push/pop, and adjusting the stack and frame pointers accordingly.

Comment 6 by capn@chromium.org, Feb 20 (3 days ago)

Project Member
Blocking: chromium:931926

Comment 7 by capn@chromium.org, Feb 20 (2 days ago)

Project Member
Cc: -senorblanco@chromium.org nicolasc...@google.com
Labels: -Priority-Medium Priority-High
Owner: senorblanco@chromium.org

Sign in to add a comment