<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi Juergen,</div><div class=""><br class=""></div><div class="">Thanks for the details!</div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On May 10, 2016, at 11:11 AM, Juergen Ributzka <<a href="mailto:juergen@apple.com" class="">juergen@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">Hi Quentin,</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">I also had to do some git archeology first :)</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">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.</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">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.</div><div class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br class=""></div><br class="" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class="">On May 9, 2016, at 4:36 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com" class="">qcolombet@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Juergen (you were in the blame list for the calling convention stuff :)),<div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">For instance, the bug that this specific commit was fixing was that we pushed AVX512 registers even when AVX512 was not used.</div></div></div></blockquote><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><br class=""></div><div class="">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?</div><div class="">I found the latter annoying to use, but I could see it is also annoying to write a different CSR list for each configuration.</div></div></div></blockquote><div class=""><br class=""></div><div class="">We usually create a separate CSR for each configuration.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><br class=""></div><div class="">Second, what is the default for AllRegs/MostRegs supposed to be?</div><div class="">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:</div><div class=""><div class="" style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;"><span class="" style="font-variant-ligatures: no-common-ligatures;"><b class="">old.s:38:2:<span class="Apple-converted-space"> </span></b></span><span class="" style="font-variant-ligatures: no-common-ligatures; color: rgb(195, 55, 32);"><b class="">error:<span class="Apple-converted-space"> </span></b></span><span class="" style="font-variant-ligatures: no-common-ligatures;"><b class="">instruction requires: AVX-512 ISA AVX-512 VL ISA</b></span></div><div class="" style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo;"><span class="" style="font-variant-ligatures: no-common-ligatures;"> vmovups %ymm31, 1008(%rsp) ## 32-byte Spill</span></div><div class="" style="margin: 0px; font-size: 11px; line-height: normal; font-family: Menlo; color: rgb(52, 189, 38);"><span class="" style="font-variant-ligatures: no-common-ligatures;"><b class=""> ^</b></span></div></div><div class=""><span class="" style="font-variant-ligatures: no-common-ligatures;"><b class=""><br class=""></b></span></div></div></div></blockquote><div class=""><br class=""></div><div class="">MostRegs was defined for the cold calling convention. AllRegs is supposed to be all registers and was originally defined for the AnyReg calling convention.</div><br class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class="">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.</div></div></div></blockquote><div class=""><br class=""></div><div class="">What is wrong with the MostRegs list?</div></div></div></blockquote><div><br class=""></div><div>It assumes the target has SSE.</div><br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><br class=""></div><div class=""><div class="">How would you recommend we fix this?</div></div></div></div></blockquote><div class=""><br class=""></div><div class="">Use a separate CSR for AVX512 (as Craig did).</div></div></div></blockquote><div><br class=""></div>Ok thanks for the confirmation. Looks like we need to audit all of CSRs list to make sure we do the right thing.</div><div><br class=""></div><div>Cheers,</div><div>-Quentin<br class=""><blockquote type="cite" class=""><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><br class=""></div><div class="">Thanks,</div><div class="">-Quentin</div></div></div></blockquote><div class=""><br class=""></div><div class="">Cheers,</div><div class="">Juergen</div><br class=""><blockquote type="cite" class=""><div class=""><div class="" style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div class=""><div class=""><div class=""><blockquote type="cite" class=""><div class="">On May 9, 2016, at 3:37 PM, Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="">Author: qcolombet<br class="">Date: Mon May 9 17:37:05 2016<br class="">New Revision: 268983<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=268983&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=268983&view=rev</a><br class="">Log:<br class="">[X86] Fix the AllRegs AVX calling convention.<br class=""><br class="">We used to list registers that were not in the AVX space. In other<br class="">words, we were pushing registers that the ISA cannot encode<br class="">(YMM16-YMM31).<br class=""><br class="">This is part of<span class="Apple-converted-space"> </span><a href="http://llvm.org/PR27481" class="">llvm.org/PR27481</a>.<br class=""><br class="">Modified:<br class=""> llvm/trunk/lib/Target/X86/X86CallingConv.td<br class=""> llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll<br class=""><br class="">Modified: llvm/trunk/lib/Target/X86/X86CallingConv.td<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CallingConv.td?rev=268983&r1=268982&r2=268983&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86CallingConv.td?rev=268983&r1=268982&r2=268983&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/lib/Target/X86/X86CallingConv.td (original)<br class="">+++ llvm/trunk/lib/Target/X86/X86CallingConv.td Mon May 9 17:37:05 2016<br class="">@@ -897,7 +897,7 @@ def CSR_32_AllRegs_SSE : CalleeSavedRegs<br class="">def CSR_64_AllRegs : CalleeSavedRegs<(add CSR_64_MostRegs, RAX, RSP,<br class=""> (sequence "XMM%u", 16, 31))>;<br class="">def CSR_64_AllRegs_AVX : CalleeSavedRegs<(sub (add CSR_64_MostRegs, RAX, RSP,<br class="">- (sequence "YMM%u", 0, 31)),<br class="">+ (sequence "YMM%u", 0, 15)),<br class=""> (sequence "XMM%u", 0, 15))>;<br class=""><br class="">// Standard C + YMM6-15<br class=""><br class="">Modified: llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll<br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll?rev=268983&r1=268982&r2=268983&view=diff" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll?rev=268983&r1=268982&r2=268983&view=diff</a><br class="">==============================================================================<br class="">--- llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll (original)<br class="">+++ llvm/trunk/test/CodeGen/X86/x86-interrupt_vzeroupper.ll Mon May 9 17:37:05 2016<br class="">@@ -1,4 +1,4 @@<br class="">-; RUN: llc -mtriple=x86_64-unknown-unknown -mattr=+avx < %s | FileCheck %s<br class="">+; RUN: llc -verify-machineinstrs -mtriple=x86_64-unknown-unknown -mattr=+avx < %s | FileCheck %s<br class=""><br class="">;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;<br class="">;; Checks that interrupt handler code does not call "vzeroupper" instruction<br class=""><br class=""><br class="">_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a><br class=""><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a></div></div></blockquote></div></div></div></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>