<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=""><br class=""><div><blockquote type="cite" class=""><div class="">On Jun 2, 2017, at 4:00 PM, Matthias Braun <<a href="mailto:mbraun@apple.com" class="">mbraun@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><blockquote type="cite" 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-size-adjust: auto; -webkit-text-stroke-width: 0px;" class=""><br class="Apple-interchange-newline">On Jun 2, 2017, at 3:46 PM, Quentin Colombet via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""><br class="">Author: qcolombet<br class="">Date: Fri Jun 2 17:46:31 2017<br class="">New Revision: 304603<br class=""><br class="">URL: <a href="http://llvm.org/viewvc/llvm-project?rev=304603&view=rev" class="">http://llvm.org/viewvc/llvm-project?rev=304603&view=rev</a><br class="">Log:<br class="">[RABasic] Properly update the LiveRegMatrix when LR splitting occur<br class=""><br class="">Prior to this patch we used to not touch the LiveRegMatrix while doing<br class="">live-range splitting. In other words, when live-range splitting was<br class="">occurring, the LiveRegMatrix was not reflecting the changes.<br class="">This is generally fine because it means the query to the LiveRegMatrix<br class="">will be conservately correct. However, when decisions are taken based on<br class="">what is going to happen on the interferences (e.g., when we spill a<br class="">register and know that it is going to be available for another one), we<br class="">might hit an assertion that the color used for the assignment is still<br class="">in use.<br class=""><br class="">This patch makes sure the changes on the live-ranges are properly<br class="">reflected in the LiveRegMatrix, so the assertions don't break.<br class="">An alternative could have been to remove the assertion, but it would<br class="">make the invariants of the code and the general reasoning more<br class="">complicated in my opnion.<br class=""><br class=""><a href="http://llvm.org/PR33057" class="">http://llvm.org/PR33057</a><br class=""><br class="">Added:<br class=""> llvm/trunk/test/CodeGen/SystemZ/RAbasic-invalid-LR-update.mir<br class="">Modified:<br class=""> llvm/trunk/lib/CodeGen/RegAllocBasic.cpp<br class=""><br class="">Modified: llvm/trunk/lib/CodeGen/RegAllocBasic.cpp<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/RegAllocBasic.cpp?rev=304603&r1=304602&r2=304603&view=diff<br class="">==============================================================================<br class="">--- llvm/trunk/lib/CodeGen/RegAllocBasic.cpp (original)<br class="">+++ llvm/trunk/lib/CodeGen/RegAllocBasic.cpp Fri Jun 2 17:46:31 2017<br class="">@@ -58,8 +58,9 @@ namespace {<br class="">/// whenever a register is unavailable. This is not practical in production but<br class="">/// provides a useful baseline both for measuring other allocators and comparing<br class="">/// the speed of the basic algorithm against other styles of allocators.<br class="">-class RABasic : public MachineFunctionPass, public RegAllocBase<br class="">-{<br class="">+class RABasic : public MachineFunctionPass,<br class="">+ public RegAllocBase,<br class="">+ private LiveRangeEdit::Delegate {<br class=""> // context<br class=""> MachineFunction *MF;<br class=""><br class="">@@ -72,6 +73,9 @@ class RABasic : public MachineFunctionPa<br class=""> // selectOrSplit().<br class=""> BitVector UsableRegs;<br class=""><br class="">+ bool LRE_CanEraseVirtReg(unsigned) override;<br class="">+ void LRE_WillShrinkVirtReg(unsigned) override;<br class="">+<br class="">public:<br class=""> RABasic();<br class=""><br class="">@@ -138,6 +142,28 @@ INITIALIZE_PASS_DEPENDENCY(LiveRegMatrix<br class="">INITIALIZE_PASS_END(RABasic, "regallocbasic", "Basic Register Allocator", false,<br class=""> false)<br class=""><br class="">+bool RABasic::LRE_CanEraseVirtReg(unsigned VirtReg) {<br class="">+ if (VRM->hasPhys(VirtReg)) {<br class="">+ LiveInterval &LI = LIS->getInterval(VirtReg);<br class="">+ Matrix->unassign(LI);<br class="">+ aboutToRemoveInterval(LI);<br class="">+ return true;<br class="">+ }<br class="">+ // Unassigned virtreg is probably in the priority queue.<br class="">+ // RegAllocBase will erase it after dequeueing.<br class="">+ return false;<br class="">+}<br class="">+<br class="">+void RABasic::LRE_WillShrinkVirtReg(unsigned VirtReg) {<br class="">+ if (!VRM->hasPhys(VirtReg))<br class="">+ return;<br class="">+<br class="">+ // Register is assigned, put it back on the queue for reassignment.<br class="">+ LiveInterval &LI = LIS->getInterval(VirtReg);<br class="">+ Matrix->unassign(LI);<br class="">+ enqueue(&LI);<br class="">+}<br class="">+<br class="">RABasic::RABasic(): MachineFunctionPass(ID) {<br class="">}<br class=""><br class="">@@ -207,7 +233,7 @@ bool RABasic::spillInterferences(LiveInt<br class=""> Matrix->unassign(Spill);<br class=""><br class=""> // Spill the extracted interval.<br class="">- LiveRangeEdit LRE(&Spill, SplitVRegs, *MF, *LIS, VRM, nullptr, &DeadRemats);<br class="">+ LiveRangeEdit LRE(&Spill, SplitVRegs, *MF, *LIS, VRM, this, &DeadRemats);<br class=""> spiller().spill(LRE);<br class=""> }<br class=""> return true;<br class="">@@ -266,7 +292,7 @@ unsigned RABasic::selectOrSplit(LiveInte<br class=""> DEBUG(dbgs() << "spilling: " << VirtReg << '\n');<br class=""> if (!VirtReg.isSpillable())<br class=""> return ~0u;<br class="">- LiveRangeEdit LRE(&VirtReg, SplitVRegs, *MF, *LIS, VRM, nullptr, &DeadRemats);<br class="">+ LiveRangeEdit LRE(&VirtReg, SplitVRegs, *MF, *LIS, VRM, this, &DeadRemats);<br class=""> spiller().spill(LRE);<br class=""><br class=""> // The live virtual register requesting allocation was spilled, so tell<br class=""><br class="">Added: llvm/trunk/test/CodeGen/SystemZ/RAbasic-invalid-LR-update.mir<br class="">URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/SystemZ/RAbasic-invalid-LR-update.mir?rev=304603&view=auto<br class="">==============================================================================<br class="">--- llvm/trunk/test/CodeGen/SystemZ/RAbasic-invalid-LR-update.mir (added)<br class="">+++ llvm/trunk/test/CodeGen/SystemZ/RAbasic-invalid-LR-update.mir Fri Jun 2 17:46:31 2017<br class="">@@ -0,0 +1,279 @@<br class="">+# RUN: llc -mtriple=s390x-linux-gnu -mcpu=z13 -run-pass=regallocbasic %s -o - | FileCheck %s<br class="">+# This test used to assert in RABasic. The problem was when we split live-ranges,<br class="">+# we were not updating the LiveRegMatrix properly and the interference calculation<br class="">+# wouldn't match what the assignment thought it could do.<br class="">+# In other words, this test case needs to trigger live-range splitting to exercise<br class="">+# the problem.<br class="">+#<br class="">+# PR33057<br class="">+--- |<br class="">+ target datalayout = "E-m:e-i1:8:16-i8:8:16-i64:64-f128:64-v128:64-a:8:16-n32:64"<br class="">+ target triple = "s390x--linux-gnu"<br class="">+ <br class="">+ define void @autogen_SD21418() #0 {<br class="">+ ret void<br class="">+ }<br class="">+ <br class="">+ attributes #0 = { "target-cpu"="z13" }<br class="">+<br class="">+...<br class="">+<br class="">+# CHECK: name: autogen_SD21418<br class="">+# Check that at least one live-range has been split<br class="">+# CHECK: id: 114, class<br class="">+---<br class="">+name: autogen_SD21418<br class="">+alignment: 2<br class="">+tracksRegLiveness: true<br class="">+registers: <br class="">+ - { id: 0, class: vr128bit }<br class="">+ - { id: 1, class: vr128bit }<br class="">+ - { id: 2, class: vr128bit }<br class="">+ - { id: 3, class: vr64bit }<br class="">+ - { id: 4, class: gr64bit }<br class="">+ - { id: 5, class: vr128bit }<br class="">+ - { id: 6, class: grx32bit }<br class="">+ - { id: 7, class: vr128bit }<br class="">+ - { id: 8, class: vr128bit }<br class="">+ - { id: 9, class: gr32bit }<br class="">+ - { id: 10, class: gr64bit }<br class="">+ - { id: 11, class: vr128bit }<br class="">+ - { id: 12, class: fp64bit }<br class="">+ - { id: 13, class: vr64bit }<br class="">+ - { id: 14, class: vr64bit }<br class="">+ - { id: 15, class: gr64bit }<br class="">+ - { id: 16, class: gr128bit }<br class="">+ - { id: 17, class: gr64bit }<br class="">+ - { id: 18, class: gr32bit }<br class="">+ - { id: 19, class: gr32bit }<br class="">+ - { id: 20, class: gr128bit }<br class="">+ - { id: 21, class: gr32bit }<br class="">+ - { id: 22, class: gr64bit }<br class="">+ - { id: 23, class: gr32bit }<br class="">+ - { id: 24, class: gr32bit }<br class="">+ - { id: 25, class: gr128bit }<br class="">+ - { id: 26, class: grx32bit }<br class="">+ - { id: 27, class: gr64bit }<br class="">+ - { id: 28, class: gr64bit }<br class="">+ - { id: 29, class: vr128bit }<br class="">+ - { id: 30, class: vr128bit }<br class="">+ - { id: 31, class: gr64bit }<br class="">+ - { id: 32, class: gr32bit }<br class="">+ - { id: 33, class: gr32bit }<br class="">+ - { id: 34, class: gr128bit }<br class="">+ - { id: 35, class: gr32bit }<br class="">+ - { id: 36, class: vr128bit }<br class="">+ - { id: 37, class: gr64bit }<br class="">+ - { id: 38, class: gr32bit }<br class="">+ - { id: 39, class: gr32bit }<br class="">+ - { id: 40, class: gr128bit }<br class="">+ - { id: 41, class: gr32bit }<br class="">+ - { id: 42, class: addr64bit }<br class="">+ - { id: 43, class: grx32bit }<br class="">+ - { id: 44, class: addr64bit }<br class="">+ - { id: 45, class: vr64bit }<br class="">+ - { id: 46, class: vr64bit }<br class="">+ - { id: 47, class: gr32bit }<br class="">+ - { id: 48, class: gr32bit }<br class="">+ - { id: 49, class: grx32bit }<br class="">+ - { id: 50, class: vr64bit }<br class="">+ - { id: 51, class: gr64bit }<br class="">+ - { id: 52, class: grx32bit }<br class="">+ - { id: 53, class: gr32bit }<br class="">+ - { id: 54, class: gr64bit }<br class="">+ - { id: 55, class: grx32bit }<br class="">+ - { id: 56, class: gr32bit }<br class="">+ - { id: 57, class: gr128bit }<br class="">+ - { id: 58, class: gr128bit }<br class="">+ - { id: 59, class: gr32bit }<br class="">+ - { id: 60, class: gr64bit }<br class="">+ - { id: 61, class: grx32bit }<br class="">+ - { id: 62, class: gr32bit }<br class="">+ - { id: 63, class: gr64bit }<br class="">+ - { id: 64, class: grx32bit }<br class="">+ - { id: 65, class: gr32bit }<br class="">+ - { id: 66, class: gr128bit }<br class="">+ - { id: 67, class: gr128bit }<br class="">+ - { id: 68, class: grx32bit }<br class="">+ - { id: 69, class: gr64bit }<br class="">+ - { id: 70, class: gr64bit }<br class="">+ - { id: 71, class: vr128bit }<br class="">+ - { id: 72, class: vr128bit }<br class="">+ - { id: 73, class: gr64bit }<br class="">+ - { id: 74, class: grx32bit }<br class="">+ - { id: 75, class: gr32bit }<br class="">+ - { id: 76, class: gr64bit }<br class="">+ - { id: 77, class: grx32bit }<br class="">+ - { id: 78, class: gr32bit }<br class="">+ - { id: 79, class: gr128bit }<br class="">+ - { id: 80, class: gr128bit }<br class="">+ - { id: 81, class: gr32bit }<br class="">+ - { id: 82, class: vr128bit }<br class="">+ - { id: 83, class: gr64bit }<br class="">+ - { id: 84, class: grx32bit }<br class="">+ - { id: 85, class: gr32bit }<br class="">+ - { id: 86, class: gr64bit }<br class="">+ - { id: 87, class: grx32bit }<br class="">+ - { id: 88, class: gr32bit }<br class="">+ - { id: 89, class: gr128bit }<br class="">+ - { id: 90, class: gr128bit }<br class="">+ - { id: 91, class: gr32bit }<br class="">+ - { id: 92, class: grx32bit }<br class="">+ - { id: 93, class: gr64bit }<br class="">+ - { id: 94, class: gr32bit }<br class="">+ - { id: 95, class: gr32bit }<br class="">+ - { id: 96, class: gr32bit }<br class="">+ - { id: 97, class: gr64bit }<br class="">+ - { id: 98, class: gr64bit }<br class="">+ - { id: 99, class: grx32bit }<br class="">+ - { id: 100, class: grx32bit }<br class="">+ - { id: 101, class: gr128bit }<br class="">+ - { id: 102, class: gr128bit }<br class="">+ - { id: 103, class: gr128bit }<br class="">+ - { id: 104, class: gr64bit }<br class="">+ - { id: 105, class: gr128bit }<br class="">+ - { id: 106, class: gr128bit }<br class="">+ - { id: 107, class: gr64bit }<br class="">+ - { id: 108, class: gr128bit }<br class="">+ - { id: 109, class: gr128bit }<br class="">+ - { id: 110, class: gr64bit }<br class="">+ - { id: 111, class: gr128bit }<br class="">+ - { id: 112, class: gr128bit }<br class="">+ - { id: 113, class: gr64bit }<br class="">+constants: <br class="">+ - id: 0<br class="">+ value: double 0xD55960F86F577076<br class="">+ alignment: 8<br class="">+body: |<br class="">+ bb.0:<br class="">+ %11 = VGBM 0<br class="">+ %43 = LHIMux 0<br class="">+ %44 = LARL %const.0<br class="">+ %45 = VL64 %44, 0, _ :: (load 8 from constant-pool)<br class="">+ <br class="">+ bb.1:<br class="">+ ADJCALLSTACKDOWN 0, 0<br class="">+ %12 = LZDR<br class="">+ %f0d = COPY %12<br class="">+ CallBRASL $fmod, killed %f0d, undef %f2d, csr_systemz, implicit-def dead %r14d, implicit-def dead %cc, implicit-def %f0d<br class="">+ ADJCALLSTACKUP 0, 0<br class="">+ KILL killed %f0d<br class="">+ <br class="">+ bb.2:<br class="">+ successors: %bb.2(0x7c000000), %bb.3(0x04000000)<br class=""></blockquote><span 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; float: none; display: inline !important;" class="">You probably could have used `-simplify-mir` to skip all those successors lines.</span><br 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=""></div></blockquote><div><br class=""></div><div>I did.</div><div>Look around, you’ll see that some of the successors haven’t been set. A bug in the simplifier?</div><div><br class=""></div><br class=""><blockquote type="cite" class=""><div class=""><br 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" 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-size-adjust: auto; -webkit-text-stroke-width: 0px;" class="">+ <br class="">+ %17 = VLGVH %11, _, 0<br class="">+ %19 = LHR %17.subreg_l32<br class="">+ undef %20.subreg_l64 = LGHI 0<br class="">+ %20 = DSGFR %20, %19<br class="">+ %22 = VLGVH %11, _, 3<br class="">+ %24 = LHR %22.subreg_l32<br class="">+ undef %25.subreg_l64 = LGHI 0<br class="">+ %25 = DSGFR %25, %24<br class="">+ %31 = VLGVH %11, _, 1<br class="">+ %33 = LHR %31.subreg_l32<br class="">+ undef %34.subreg_l64 = LGHI 0<br class="">+ %34 = DSGFR %34, %33<br class="">+ %37 = VLGVH %11, _, 2<br class="">+ %39 = LHR %37.subreg_l32<br class="">+ undef %40.subreg_l64 = LGHI 0<br class="">+ %40 = DSGFR %40, %39<br class="">+ CHIMux %43, 0, implicit-def %cc<br class="">+ BRC 14, 6, %bb.2, implicit killed %cc<br class="">+ J %bb.3<br class="">+ <br class="">+ bb.3:<br class="">+ successors: %bb.1(0x7c000000), %bb.4(0x04000000)<br class="">+ <br class="">+ WFCDB undef %46, %45, implicit-def %cc<br class="">+ %48 = IPM implicit killed %cc<br class="">+ %48 = AFIMux %48, 268435456, implicit-def dead %cc<br class="">+ %6 = RISBMux undef %6, %48, 31, 159, 35<br class="">+ WFCDB undef %50, %45, implicit-def %cc<br class="">+ BRC 15, 6, %bb.1, implicit killed %cc<br class="">+ J %bb.4<br class="">+ <br class="">+ bb.4:<br class="">+ %36 = VLVGP %25.subreg_l64, %25.subreg_l64<br class="">+ %36 = VLVGH %36, %20.subreg_l32, _, 0<br class="">+ %36 = VLVGH %36, %34.subreg_l32, _, 1<br class="">+ dead %36 = VLVGH %36, %40.subreg_l32, _, 2<br class="">+ %4 = LG undef %42, 0, _ :: (load 8 from `i64* undef`)<br class="">+ undef %57.subreg_h64 = LLILL 0<br class="">+ undef %66.subreg_h64 = LLILL 0<br class="">+ undef %79.subreg_h64 = LLILL 0<br class="">+ undef %89.subreg_h64 = LLILL 0<br class="">+ %92 = LHIMux 0<br class="">+ <br class="">+ bb.5:<br class="">+ <br class="">+ bb.6:<br class="">+ successors: %bb.6, %bb.7<br class="">+ <br class="">+ %51 = VLGVH undef %7, _, 0<br class="">+ %53 = LLHRMux %51.subreg_l32<br class="">+ %54 = VLGVH undef %1, _, 0<br class="">+ %57.subreg_l32 = LLHRMux %54.subreg_l32<br class="">+ %58 = COPY %57<br class="">+ %58 = DLR %58, %53<br class="">+ %60 = VLGVH undef %7, _, 3<br class="">+ %62 = LLHRMux %60.subreg_l32<br class="">+ %63 = VLGVH undef %1, _, 3<br class="">+ %66.subreg_l32 = LLHRMux %63.subreg_l32<br class="">+ %67 = COPY %66<br class="">+ %67 = DLR %67, %62<br class="">+ %73 = VLGVH undef %7, _, 1<br class="">+ %75 = LLHRMux %73.subreg_l32<br class="">+ %76 = VLGVH undef %1, _, 1<br class="">+ %79.subreg_l32 = LLHRMux %76.subreg_l32<br class="">+ %80 = COPY %79<br class="">+ %80 = DLR %80, %75<br class="">+ %83 = VLGVH undef %7, _, 2<br class="">+ %85 = LLHRMux %83.subreg_l32<br class="">+ %86 = VLGVH undef %1, _, 2<br class="">+ %89.subreg_l32 = LLHRMux %86.subreg_l32<br class="">+ %90 = COPY %89<br class="">+ %90 = DLR %90, %85<br class="">+ CHIMux %92, 0, implicit-def %cc<br class="">+ BRC 14, 6, %bb.7, implicit killed %cc<br class="">+ J %bb.6<br class="">+ <br class="">+ bb.7:<br class="">+ successors: %bb.6(0x7c000000), %bb.8(0x04000000)<br class="">+ <br class="">+ CGHI undef %93, 0, implicit-def %cc<br class="">+ %96 = IPM implicit killed %cc<br class="">+ CGHI undef %97, 0, implicit-def %cc<br class="">+ BRC 14, 6, %bb.6, implicit killed %cc<br class="">+ <br class="">+ bb.8:<br class="">+ successors: %bb.5(0x7c000000), %bb.9(0x04000000)<br class="">+ <br class="">+ CHIMux %6, 0, implicit-def %cc<br class="">+ %10 = LLILL 41639<br class="">+ dead %10 = LOCGR %10, %4, 14, 6, implicit killed %cc<br class="">+ CHIMux %92, 0, implicit-def %cc<br class="">+ BRC 14, 6, %bb.5, implicit killed %cc<br class="">+ J %bb.9<br class="">+ <br class="">+ bb.9:<br class="">+ %82 = VLVGP %67.subreg_h64, %67.subreg_h64<br class="">+ %82 = VLVGH %82, %58.subreg_hl32, _, 0<br class="">+ %82 = VLVGH %82, %80.subreg_hl32, _, 1<br class="">+ dead %82 = VLVGH %82, %90.subreg_hl32, _, 2<br class="">+ %96 = AFIMux %96, 1879048192, implicit-def dead %cc<br class="">+ %96 = SRL %96, _, 31<br class="">+ dead %11 = VLVGF %11, %96, _, 1<br class="">+ %100 = LHIMux 0<br class="">+ <br class="">+ bb.10:<br class="">+ successors: %bb.10(0x7c000000), %bb.11(0x04000000)<br class="">+ <br class="">+ CHIMux %100, 0, implicit-def %cc<br class="">+ BRC 14, 6, %bb.10, implicit killed %cc<br class="">+ J %bb.11<br class="">+ <br class="">+ bb.11:<br class="">+ Return<br class="">+<br class="">+...<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="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</blockquote></div></blockquote></div><br class=""></body></html>