<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;">Done.<div><br></div><div>-bw</div><div><br><div style=""><div>On Aug 15, 2014, at 4:07 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: 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;">----- Original Message -----<br><blockquote type="cite">From: "İsmail Dönmez" <<a href="mailto:ismail@donmez.ws">ismail@donmez.ws</a>><br>To: "Bill Schmidt" <<a href="mailto:wschmidt@linux.vnet.ibm.com">wschmidt@linux.vnet.ibm.com</a>><br>Cc: "llvm-commits" <<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a>><br>Sent: Thursday, August 14, 2014 10:47:28 PM<br>Subject: Re: [llvm] r215685 - [PPC64] Add missing dependency on X2 to<span class="Apple-tab-span" style="white-space: pre;"> </span>LDinto_toc.<br><br>Cc'ing Bill Wendling. This should be in 3.5 because at least on<br>(open)SUSE we bootstrap clang on PPC64.<br></blockquote><br>Bill,<br><br>This is approved. Please pull this commit into the release branch! (it fixes self-hosting).<br><br>Thanks again,<br>Hal<br><br><blockquote type="cite"><br>On Aug 15, 2014 4:59 AM, "Bill Schmidt" < <a href="mailto:wschmidt@linux.vnet.ibm.com">wschmidt@linux.vnet.ibm.com</a><br><blockquote type="cite">wrote:<br></blockquote><br><br>Author: wschmidt<br>Date: Thu Aug 14 20:25:26 2014<br>New Revision: 215685<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=215685&view=rev">http://llvm.org/viewvc/llvm-project?rev=215685&view=rev</a><br>Log:<br>[PPC64] Add missing dependency on X2 to LDinto_toc.<br><br>The LDinto_toc pattern has been part of 64-bit PowerPC for a long<br>time, and represents loading from a memory location into the TOC<br>register (X2). However, this pattern doesn't explicitly record that<br>it modifies that register. This patch adds the missing dependency.<br><br>It was very surprising to me that this has never shown up as a<br>problem<br>in the past, and that we only saw this problem recently in a single<br>scenario when building a self-hosted clang. It turns out that in most<br>cases we have another dependency present that keeps the LDinto_toc<br>instruction tied in place. LDinto_toc is used for TOC restore<br>following a call site, so this is a typical sequence:<br><br>BCTRL8 <regmask>, %CTR8<imp-use>, %RM<imp-use>, %X3<imp-use>,<br>%X12<imp-use>, %X1<imp-def>, ...<br>LDinto_toc 24, %X1<br>ADJCALLSTACKUP 96, 0, %R1<imp-def>, %R1<imp-use><br><br>Because the LDinto_toc is inserted prior to the ADJCALLSTACKUP, there<br>is a natural anti-dependency between the two that keeps it in place.<br><br>Therefore we don't usually see a problem. However, in one particular<br>case, one call is followed immediately by another call, and the<br>second<br>call requires a parameter that is a TOC-relative address. This is the<br>code sequence:<br><br>BCTRL8 <regmask>, %CTR8<imp-use>, %RM<imp-use>, %X3<imp-use>,<br>%X4<imp-use>, %X5<imp-use>, %X12<imp-use>, %X1<imp-def>, ...<br>LDinto_toc 24, %X1<br>ADJCALLSTACKUP 96, 0, %R1<imp-def>, %R1<imp-use><br>ADJCALLSTACKDOWN 96, %R1<imp-def>, %R1<imp-use><br>%vreg39<def> = ADDIStocHA %X2, <ga:@.str>; G8RC_and_G8RC_NOX0:%vreg39<br>%vreg40<def> = ADDItocL %vreg39<kill>, <ga:@.str>; G8RC:%vreg40<br>G8RC_and_G8RC_NOX0:%vreg39<br><br>Note that the back-to-back stack adjustments are the same size! The<br>back end is smart enough to recognize this and optimize them away:<br><br>BCTRL8 <regmask>, %CTR8<imp-use>, %RM<imp-use>, %X3<imp-use>,<br>%X4<imp-use>, %X5<imp-use>, %X12<imp-use>, %X1<imp-def>, ...<br>LDinto_toc 24, %X1<br>%vreg39<def> = ADDIStocHA %X2, <ga:@.str>; G8RC_and_G8RC_NOX0:%vreg39<br>%vreg40<def> = ADDItocL %vreg39<kill>, <ga:@.str>; G8RC:%vreg40<br>G8RC_and_G8RC_NOX0:%vreg39<br><br>Now there is nothing to prevent the ADDIStocHA instruction from<br>moving<br>ahead of the LDinto_toc instruction, and because of the longest-path<br>heuristic, this is what happens.<br><br>With the accompanying patch, %X2 is represented as an implicit def:<br><br>BCTRL8 <regmask>, %CTR8<imp-use>, %RM<imp-use>, %X3<imp-use>,<br>%X4<imp-use>, %X5<imp-use>, %X12<imp-use>, %X1<imp-def>, ...<br>LDinto_toc 24, %X1, %X2<imp-def,dead><br>ADJCALLSTACKUP 96, 0, %R1<imp-def,dead>, %R1<imp-use><br>ADJCALLSTACKDOWN 96, %R1<imp-def,dead>, %R1<imp-use><br>%vreg39<def> = ADDIStocHA %X2, <ga:@.str>; G8RC_and_G8RC_NOX0:%vreg39<br>%vreg40<def> = ADDItocL %vreg39<kill>, <ga:@.str>; G8RC:%vreg40<br>G8RC_and_G8RC_NOX0:%vreg39<br><br>So now when the two stack adjustments are removed, ADDIStocHA is<br>prevented from being moved above LDinto_toc.<br><br>I have not yet created a test case for this, because the original<br>failure occurs on a relatively large function that needs reduction.<br>However, this is a fairly serious bug, despite its infrequency, and I<br>wanted to get this patch onto the list as soon as possible so that it<br>can be considered for a 3.5 backport. I'll work on whittling down a<br>test case.<br><br>Have we missed the boat for 3.5 at this point?<br><br>Thanks,<br>Bill<br><br>Modified:<br>llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td<br><br>Modified: llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td<br>URL:<br><a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td?rev=215685&r1=215684&r2=215685&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td?rev=215685&r1=215684&r2=215685&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td (original)<br>+++ llvm/trunk/lib/Target/PowerPC/PPCInstr64Bit.td Thu Aug 14<br>20:25:26 2014<br>@@ -802,7 +802,7 @@ def LDtocCPT: Pseudo<(outs g8rc:$rD), (i<br>[(set i64:$rD,<br>(PPCtoc_entry tconstpool:$disp, i64:$reg))]>, isPPC64;<br><br>-let hasSideEffects = 1, isCodeGenOnly = 1, RST = 2 in<br>+let hasSideEffects = 1, isCodeGenOnly = 1, RST = 2, Defs = [X2] in<br>def LDinto_toc: DSForm_1<58, 0, (outs), (ins memrix:$src),<br>"ld 2, $src", IIC_LdStLD,<br>[(PPCload_toc ixaddr:$src)]>, isPPC64;<br><br><br>_______________________________________________<br>llvm-commits mailing list<br>llvm-commits@cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br><br></blockquote><br>--<span class="Apple-converted-space"> </span><br>Hal Finkel<br>Assistant Computational Scientist<br>Leadership Computing Facility<br>Argonne National Laboratory</div></blockquote></div><br></div></body></html>