<div dir="ltr">Patches 0012 ~ 0018 look ok. As usual, small comment on new functions.<div><br></div><div>Specific comments follow...<br><div><br></div><div>On 0012:</div><div><br></div><div><div>+      // TODO: think if this can actually happen...</div>
<div>+      if (*M == ~0u)</div><div>+        PrintFatalError("Invalid regunitlanemask");</div></div><div><br></div><div>that could be an assert...<br><div><br></div><div>On 0014, assign/unassign/checkRegUnitInterference have very similar structures, but I can't think of a way to common up that code without introducing virtual dispatch or function pointers. :( Maybe it's ok the way it is.</div>
<div><br></div><div>On 0015:</div><div><br></div><div><div>+    if (Mask != MaxMask) {</div><div>+      // No need for further tracking if complete subranges are missing.</div><div>+      CancelAllKills = true;</div><div>
+    }</div><div><br></div><div>shouldn't this be (Mask == MaxMask)? As in: all masks join up to the total mask, thus, cancel all kills.</div><div><br></div><div>On 0016:</div></div></div><div><br></div><div><div>+        // when tracking subregister liveness the main range must start new</div>
<div>+        // values on partial register writes, even if there is no read.</div><div>+        if (!MRI->tracksSubRegLiveness() || LaneMask != 0 || !hasSubRegDef) {</div></div><div><br></div><div>For my education, you're making sure there isn't a write on a sub-register, via a super-register's lane. Ie., in this case, the main range must create a write to the specific sub-register, so you can keep score of its liveness, right?</div>
<div><br></div><div>On 0018:</div><div><br></div><div>I'm thinking we could add all 17 previous patches, let the buildbots run and stabilize (if they get red), then, when everything is fine, enable them by default on ARM. That'll give us at least two stages in which to fix problems.</div>
<div><br></div><div>If you're feeling adventurous, you could separate your patch into functional groups and apply them a group at a time, and see if there's any bots or additional tests breaking, but I personally don't think that's necessary.</div>
<div><br></div><div>A general comment about all patches, thank you for working on this. The code is clear and you have added a lot of comments in between the lines, making my life a lot easier to review, and helped me learn about how the live ranges are computed, which will help other people read the code in the future, too.</div>
<div><br></div><div>cheers,</div></div><div>--renato</div></div>