<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="">This looks good. Thanks for the thorough description.<div class=""><br class=""></div><div class="">The fix itself is pretty obviously correct. I had some concerns that there might be other issues exposed in a case like this where there are very few instructions remaining to split. I generally expect the split-the-basic-block situation to come up when there is a really large block, but in this case the block is small. The reason the constant cannot be placed in this test is that we have a load with a very small offset field and the block is followed by a large constant pool. Our rules for convergence require that constants be placed at the end of an existing pool, which in this case is out of range. Akira and I discussed this in person, and we couldn’t find any other problems. Basically you can always end up with the fall-back of splitting immediately after the load instruction.</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Oct 16, 2014, at 4:19 PM, Akira Hatanaka <<a href="mailto:ahatanak@gmail.com" class="">ahatanak@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">The attached patch fixes a bug in ARM's constant island pass. The bug is in ARMConstantIslandPass.cpp:1307-1314 where the upper bound of the new water split point is computed:</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">// This <span style="color:rgb(255,255,255);background-color:rgb(0,0,0)" class="">could poi</span>nt off the end of the block if we've already got constant<br class=""></div></div><div class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">// pool entries following this block; only the last one is in the water list.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">// Back past any possible branches (allow for a conditional and a maximally</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">// long unconditional).</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">if (BaseInsertOffset + 8 >= UserBBI.postOffset()) {</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> BaseInsertOffset = UserBBI.postOffset() - UPad - 8;</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> DEBUG(dbgs() << format("Move inside block: %#x\n", BaseInsertOffset));</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">}</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><div class="">The split point is supposed to be somewhere between the machine instruction that loads from the constant pool entry and the end of the basic block, before branch instructions. The code above is fine if the basic block is large enough and there are a sufficient number of instructions following the machine instruction. However, if the machine instruction is near the end of the basic block, BaseInsertOffset can point to the machine instruction or another instruction that precedes it, and this can lead to convergence failure (see the example below).</div><div class=""><br class=""></div><div class="">The attached patch fixes this bug by ensuring BaseInsertOffset is larger than the offset of the instruction following the constant-loading instruction.</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><p style="margin:0px;font-size:11px;font-family:Menlo" class=""></p><div style="color:rgb(34,34,34);font-family:arial;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255)" class=""></div><div class=""><br class="webkit-block-placeholder"></div><div style="color:rgb(34,34,34);font-family:arial;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;line-height:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255)" class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">- BaseInsertOffset = UserBBI.postOffset() - UPad - 8;</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">+ BaseInsertOffset =<br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">+ std::max(UserBBI.postOffset() - UPad - 8,</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">+ UserOffset + TII->GetInstSizeInBytes(UserMI) + 1);</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span style="font-family:arial;font-size:small" class=""><a href="rdar://problem/18581150" class="">rdar://problem/18581150</a></span><br class=""></div></div></div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">This is how the bug can lead to convergence failure:</div><div class=""><br class=""></div><div class="">1. Instruction VLDRS is too far from the constant pool entry cp#136 in BB#189. A new water has to be created in BB#324 after VLDRS.</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#189: Align 2 (4 bytes)</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>CONSTPOOL_ENTRY 345, <cp#136>, 4</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">...</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#323: Align 2 (4 bytes)</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>CONSTPOOL_ENTRY 479, <cp#156>, 4</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#324: derived from LLVM BB %bb35</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Predecessors according to CFG: BB#187</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%D0<def> = VORRd %D9, %D9, pred:14, pred:%noreg, %S18<imp-use></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%S6<def> = VLDRS <cp#345>, 0, pred:14, pred:%noreg, %D3<imp-def>; mem:LD4[ConstantPool]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%D5<def> = VLDRD <cp#346>, 0, pred:14, pred:%noreg; mem:LD8[ConstantPool]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>t2STRi12 %R1<kill>, %R10, 0, pred:14, pred:%noreg; mem:ST4[%arg14]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>t2B <BB#342>, pred:14, pred:%noreg</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Successors according to CFG: BB#342</div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">2. However, new water (BB#325) is created before VLDRS and cp#136 is moved there, because VLDRS is too close to the end of its parent basic block. This causes all the constant pool entries between BB#189 and BB#323 to be moved after constant pool entry cp#136, because they are now out of range, and this in turn causes cp#136 to be out of range again.</div><div class=""><br class=""></div><div class=""><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#324: derived from LLVM BB %bb35</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Predecessors according to CFG: BB#187</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%D0<def> = VORRd %D9, %D9, pred:14, pred:%noreg, %S18<imp-use></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>t2B <BB#461>, pred:14, pred:%noreg</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Successors according to CFG: BB#461</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#325: Align 2 (4 bytes)</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>CONSTPOOL_ENTRY 480, <cp#136>, 4</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class="">...</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#460: Align 2 (4 bytes)</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>CONSTPOOL_ENTRY 615, <cp#156>, 4</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px;" class=""><br class=""></div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class="">BB#461: derived from LLVM BB %bb35</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Predecessors according to CFG: BB#324</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%S6<def> = VLDRS <cp#480>, 0, pred:14, pred:%noreg, %D3<imp-def>; mem:LD4[ConstantPool]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>%D5<def> = VLDRD <cp#481>, 0, pred:14, pred:%noreg; mem:LD8[ConstantPool]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>t2STRi12 %R1<kill>, %R10, 0, pred:14, pred:%noreg; mem:ST4[%arg14]</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""><span class="" style="white-space:pre"> </span>t2B <BB#479>, pred:14, pred:%noreg</div><div style="margin: 0px; font-size: 11px; font-family: Menlo;" class=""> Successors according to CFG: BB#479</div></div></div></div>
<span id="cid:6318BAA7-6E3C-4B28-A1E9-6672DDD256C1@apple.com"><constant-island1.patch></span>_______________________________________________<br class="">llvm-commits mailing list<br class=""><a href="mailto:llvm-commits@cs.uiuc.edu" class="">llvm-commits@cs.uiuc.edu</a><br class="">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br class=""></div></blockquote></div><br class=""></div></body></html>