<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Aug 23, 2013, at 3:11 AM, Robert Lytton <<a href="mailto:robert@xmos.com">robert@xmos.com</a>> wrote:</div><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;">(this picks up from the thread in llvmdev: "PrescheduleNodesWithMultipleUses() causing failure in PickNodeToScheduleBottomUp() ???")<br><br>PrescheduleNodesWithMultipleUses() is a function that tries to transform a DAG where nodes have multiple uses, with the aim of reducing the live range.<br>(see comment in lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp)<br><br>However, it does not check to see if callResources will be affected by the transformation.<br><br><br>The problem<br>========<br> Take the following correctly scheduled DAG (arrow to predecessor):<br><font face="Courier New"><br> SetUp2 SetUp1<br> ^ ^<br> | |<br> | |<br> Destroy2---->PredSU <----SU<br> ^ ^ ^<br> | | |<br> | | |<br> ----------- | ---------<br> | | |<br> Destroy1<br> ^<br> |<br></font><br> In this example there are two successors of 'PredSU' with type<br> getCallFrameDestroyOpcode (Destroy) and one is a successor of the other.<br> Taking the successor of the two Destroys (Destroy1), note that it's<br> matching getCallFrameSetupOpcode (Setup1) is a predecessor of 'SU'.<br> In this situation, re-routing the dependency on 'PredSU' through 'SU' will<br> cause a dead lock Viz:<br><br><font face="Courier New"> SetUp2 PredSU SetUp1<br> ^ ^ ^<br> | | |<br> | | |<br> Destroy2-----> SU -------<br> ^ ^ ^<br> | | |<br> | | |<br> ----------- | |<br> | | |<br> Destroy1<br> ^<br> |<br></font><br><font face="Tahoma" size="2">The new function callResourceDeadLockDanger()</font><span class="Apple-converted-space"> </span>will check for this situation<br>and prevent PrescheduleNodesWithMultipleUses() from making the transformation<br>if it will cause a callResource dead lock.<br></div></div></div></blockquote><div><br></div><div>Robert,</div><div><br></div><div>I appreciate that you put a lot of care into this patch. Your comments are amazing. But do you absolutely need to call PrescheduleNodesWithMultipleUses? x86_64 and arm don't use it, and it is about to be deprecated.</div><div><br></div><div>All targets should move to source order SD scheduling:</div><div><br></div><div><target>TargetLowering::resetOperationActions() {</div><div>...</div><div> setSchedulingPreference(Sched::Source);</div><div><br></div><div>If instruction scheduling is required, the target can enable MachineScheduler:</div><div><br></div><div>bool <target>SubTargetInfo::enableMachineScheduler() const { return true; }</div><div><br></div><div>If for some reason the source order scheduler won't work in the short term, you can choose one of these SD schedulers that don't run PrescheduleNodesWithMultipleUses:</div><div><br></div><div>setSchedulingPreference(Sched::ILP);</div><div>setSchedulingPreference(Sched::Hybrid);</div><div><br></div><div>For educational purposes I'll try to answer your questions below (although the only good fix I can see is deleting PrescheduleNodesWithMultipleUses()).</div><div><br></div><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;">Outstanding issues<br>============<br><br>1. Is it too aggressive in searching predecessors and successors?<br> Should the algorithm give up and assume the worst if the depth of search reaches a predefined limit?<br> Can someone confirm that the extra processing time is not onerous.<br></div></div></div></blockquote><div><br></div>Walking all paths in the DAG is exponential. In a case like this, a search limit is good. Otherwise you need to maintain a visited set.</div><div><br></div><div>I also think recursive DAG walking is very bad form. LLVM runtime compilers don’t want to see stack overflow.</div><div><br><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;">2. Should the initial search for 'SetUp1' and 'Destroy1' only search along chains? viz conditional upon II->isCtrl()<br> This will reduce the search space, but are getCallFrameSetupOpcode & getCallFrameDestroyOpcode always 'chained'?<br> (Later searches for Destroy2 need to check all predecessors)<br></div></div></div></blockquote><div><br></div><div>Yes, the call sequence nodes are chained. Not all nodes within the sequence are chained. If a node in the call sequence has a chain edge, Setup/Destroy will be reachable in that direction.</div><br><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;">3. What is the best way to construct the test case?<br> Using an IR as input does not guarantee the required DAG will be output for testing.<br> The test IR only produces the correct DAG when built for the XCore target.<br> (see attached test case)<br></div></div></div></blockquote><div><br></div><div>That’s because other targets don’t call PrescheduleNodesWithMultipleUses.</div><div><br></div><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;">4. Where should the test be placed?<br> Currently I have placed it under test/CodeGen/XCore but it relates to lib/CodeGen/SelectionDAG/ScheduleDAGRRList.cpp<br></div></div></div></blockquote><div><br></div><div>That’s fine. Generic test cases are better unless you need to CHECK the disassembly.</div><div><br></div><div>-Andy</div><br><blockquote type="cite"><div style="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; direction: ltr; font-family: Tahoma; font-size: 10pt;"><div style="font-family: 'Times New Roman'; font-size: 16px;"><div style="direction: ltr; font-family: Tahoma; font-size: 10pt;"><br>Thank you<br><br>Robert<br><br></div></div></div><span><PatchCallResourceDeadLock></span><span style="font-family: Helvetica; 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; float: none; display: inline !important;">_______________________________________________</span><br style="font-family: Helvetica; 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;"><span style="font-family: Helvetica; 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; float: none; display: inline !important;">llvm-commits mailing list</span><br style="font-family: Helvetica; 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;"><a href="mailto:llvm-commits@cs.uiuc.edu" style="font-family: Helvetica; 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;">llvm-commits@cs.uiuc.edu</a><br style="font-family: Helvetica; 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;"><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" style="font-family: Helvetica; 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;">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a></blockquote></div><br></body></html>