Page MenuHomePhabricator

X86CallFrameOptimization: Bail on win64cc calls
AbandonedPublic

Authored by zvi on Jan 17 2018, 6:09 AM.

Details

Summary

This transform is not legal for Win64. We already check if MF is
not win64cc, but we also need to check that no transform is applied
on a win64cc call site.
Unfortunately, there is no straightforward way to check the callee's CC,
so we resort to checking the register mask vector for equality to the
Win64-specific entry.

Fixes pr35814

Diff Detail

Event Timeline

zvi created this revision.Jan 17 2018, 6:09 AM
zvi added a comment.Jan 17 2018, 6:11 AM

I would appreciate suggestions for alternative solutions.

rnk added a comment.Jan 17 2018, 9:52 AM

Here is the MI in question:

# *** IR Dump Before X86 Optimize Call Frame ***:
# Machine code for function pr35814: IsSSA, TracksLiveness

%bb.0: derived from LLVM BB %entry
        %0:gr32 = MOV32rm %rip, 1, %noreg, @ff, %noreg; mem:LD4[@ff](dereferenceable) GR32:%0
        %1:gr32 = MOV32rm %rip, 1, %noreg, @e, %noreg; mem:LD4[@e](dereferenceable) GR32:%1
        ADJCALLSTACKDOWN64 40, 0, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit-def dead %ssp, implicit %rsp, implicit %ssp
        %2:gr64 = MOV64rm %rip, 1, %noreg, @d + 16, %noreg; mem:LD8[<unknown>] GR64:%2
        %3:gr64 = COPY %rsp; GR64:%3
        MOV64mr %3, 1, %noreg, 48, %noreg, killed %2; mem:ST8[<unknown>] GR64:%3,%2
        %4:gr64 = MOV64rm %rip, 1, %noreg, @d + 8, %noreg; mem:LD8[<unknown>] GR64:%4
        MOV64mr %3, 1, %noreg, 40, %noreg, killed %4; mem:ST8[<unknown>] GR64:%3,%4
        %5:gr64 = MOV64rm %rip, 1, %noreg, @d, %noreg; mem:LD8[<unknown>] GR64:%5
        MOV64mr %3, 1, %noreg, 32, %noreg, killed %5; mem:ST8[<unknown>] GR64:%3,%5
        %6:gr32 = MOV32ri64 @d; GR32:%6
        %7:gr64 = SUBREG_TO_REG 0, killed %6, sub_32bit; GR64:%7 GR32:%6
        %rcx = COPY %7; GR64:%7
        %edx = COPY %0; GR32:%0
        %r8d = COPY %1; GR32:%1
        %r9 = COPY %7; GR64:%7
        CALL64pcrel32 @g, <regmask %bh %bl %bp %bpl %bx %di %dil %ebp %ebx %edi %esi %rbp %rbx %rdi %rsi %si %sil %r12 %r13 %r14 %r15 %xmm6 %xmm7 %xmm8 %xmm9 %xmm10 %xmm11 %xmm12 %xmm13 %xmm14 %xmm15 %r12b %r13b and 10 more...>, implicit %rsp, implicit %ssp, implicit %rcx, implicit %edx, implicit %r8d, implicit %r9, implicit-def %rsp, implicit-def %ssp
        ADJCALLSTACKUP64 40, 0, implicit-def dead %rsp, implicit-def dead %eflags, implicit-def dead %ssp, implicit %rsp, implicit %ssp
        RET 0

# End machine code for function pr35814.

Isn't this MI buggy? We're adjusting SP down by 40 bytes and storing to SP+48, which could overwrite data. I think the assert is valid.

zvi added a comment.Jan 17 2018, 1:48 PM
In D42171#978790, @rnk wrote:

Isn't this MI buggy? We're adjusting SP down by 40 bytes and storing to SP+48, which could overwrite data. I think the assert is valid.

There are a couple of troubling issues seen here:

  1. The caller is corrupting its own return address
  2. byval is not honored - shouldn't the structs that are passed 'byval' be passed as copies?

This patch does not address these issues.

rnk added a comment.Jan 17 2018, 2:38 PM

I think the problem is that passing aggregates to ms_abi functions in wine is broken. It generates invalid MIR. There's a bug for that somewhere (http://llvm.org/PR31362). I'd just dupe this bug against that and leave it alone. As long as the call optimization code doesn't optimize call sequences that have gaps it will naturally ignore win64cc functions.

zvi abandoned this revision.Apr 11 2018, 2:01 PM