[llvm] r268983 - [X86] Fix the AllRegs AVX calling convention.

Tue May 10 11:22:29 PDT 2016

Should 32-bit X86_INTR also save the whole YMM and ZMM register when AVX or
AVX512 is enabled? Currently it only saves the XMM portions.

On Tue, May 10, 2016 at 11:11 AM, Juergen Ributzka via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Hi Quentin,
>
> I also had to do some git archeology first :)
>
> CSR_64_AllRegs/CSR_64_AllRegs_AVX were originally introduced for the
> AnyReg calling convention, which could only be used by the stackmap and
> patchpoint intrinsics. Functions cannot uses this calling convention and
> therefore there shouldn’t be any spill code generated. Looks like
> CSR_64_AllRegs/CSR_64_AllRegs_AVX are now also used by the X86_INTR calling
> convention.
>
> Anyways, getCalleeSaved registers is already checking the current
> configuration and returns the appropriate callee save register list. The
> CSR_64_AllRegs_AVX was broken and we should return a separate one for
> AVX512. Craig already added CSR_64_AllRegs_AVX512 for the X86_INTR calling
> convention in r269018. We can use the same CSR for the AnyReg calling
> convention.
>
>
> On May 9, 2016, at 4:36 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>
> Hi Juergen (you were in the blame list for the calling convention stuff
> :)),
>
> I was doing a bit of archeology with the calling convention stuff on X86,
> because I found some bugs with the machine verifier and I wanted to double
> check the other calling convention to make sure they are correct and I need
> your help to figure out what was the intent for each of those classes.
>
> Basically, the problem is that we have a bunch of callee saved registers
> that we saved that may not actually be supported by the current
> configuration.
>
> For instance, the bug that this specific commit was fixing was that we
> pushed AVX512 registers even when AVX512 was not used.
>
>
> First, regarding the design, were the returned CSRs supposed to be
> directly usable or do users of the getCalleeSaved need to intersect with
> what the current configuration has?
> I found the latter annoying to use, but I could see it is also annoying to
> write a different CSR list for each configuration.
>
>
> We usually create a separate CSR for each configuration.
>
>
> Second, what is the default for AllRegs/MostRegs supposed to be?
> Right now, it seems to assume we have AVX512 and such, thus it is pretty
> easy to end up with an assembly file having encoding problems like this:
> *old.s:38:2: **error: **instruction requires: AVX-512 ISA AVX-512 VL ISA*
>  vmovups %ymm31, 1008(%rsp) ## 32-byte Spill
> * ^*
>
>
> MostRegs was defined for the cold calling convention. AllRegs is supposed
> to be all registers and was originally defined for the AnyReg calling
> convention.
>
> From what I saw, it seems to me that the sets of CSRs were supposed to be
> directly usable, but allregs, mostregs, and maybe others, do not follow
> that direction.
>
>
> What is wrong with the MostRegs list?
>
>
> How would you recommend we fix this?
>
>
> Use a separate CSR for AVX512 (as Craig did).
>
>
> Thanks,
> -Quentin
>
>
> Cheers,
> Juergen
>
> On May 9, 2016, at 3:37 PM, Quentin Colombet via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
> Author: qcolombet
> Date: Mon May  9 17:37:05 2016
> New Revision: 268983
>
> URL: http://llvm.org/viewvc/llvm-project?rev=268983&view=rev
> Log:
> [X86] Fix the AllRegs AVX calling convention.
>
> We used to list registers that were not in the AVX space. In other
> words, we were pushing registers that the ISA cannot encode
> (YMM16-YMM31).
>
> This is part of llvm.org/PR27481.
>
> Modified:
>    llvm/trunk/lib/Target/X86/X86CallingConv.td
>    llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll
>
> Modified: llvm/trunk/lib/Target/X86/X86CallingConv.td
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CallingConv.td?rev=268983&r1=268982&r2=268983&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Target/X86/X86CallingConv.td (original)
> +++ llvm/trunk/lib/Target/X86/X86CallingConv.td Mon May  9 17:37:05 2016
> @@ -897,7 +897,7 @@ def CSR_32_AllRegs_SSE : CalleeSavedRegs
> def CSR_64_AllRegs     : CalleeSavedRegs<(add CSR_64_MostRegs, RAX, RSP,
>                                               (sequence "XMM%u", 16, 31))>;
> def CSR_64_AllRegs_AVX : CalleeSavedRegs<(sub (add CSR_64_MostRegs, RAX,
> RSP,
> -                                                   (sequence "YMM%u", 0,
> 31)),
> +                                                   (sequence "YMM%u", 0,
> 15)),
>                                               (sequence "XMM%u", 0, 15))>;
>
> // Standard C + YMM6-15
>
> Modified: llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll?rev=268983&r1=268982&r2=268983&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll Mon May  9
> 17:37:05 2016
> @@ -1,4 +1,4 @@
> -; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+avx < %s | FileCheck %s
> +; RUN: llc -verify-machineinstrs -mtriple=x86_64-unknown-unknown
> -mattr=+avx < %s | FileCheck %s
>
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> ;; Checks that interrupt handler code does not call "vzeroupper"
> instruction
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>

-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160510/64f110ab/attachment.html>