use register iterators that include self to reduce code duplication in CriticalAntiDepBreaker

[spatel]

Summary

This patch addresses 2 FIXME comments that I added to CriticalAntiDepBreaker while fixing PR20020.

I've initialized an MCSubRegIterator and an MCRegAliasIterator to include the self reg.

Assuming that works as advertised, there should be functional difference with this patch, just less code.

[spatel]

Ping.

[atrick]

Looks like a good cleanup. Thanks.

Although the assert looks like nonsense to me. Can you see any purpose for it? Please either explain it or remove it.

[spatel]

Thanks for looking this over!
Yes, those asserts look bogus - the only way they could fire is if Count is ~0u since we're setting both Kills and Defs right ahead of the asserts. I'll remove them.

[spatel]

Closed by commit rL214973 (authored by @spatel).

llvm/trunk/lib/CodeGen/CriticalAntiDepBreaker.cpp

for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
       if (!MO.isDef()) continue;
 
       // If we've already marked this reg as unchangeable, carry on.
       if (KeepRegs.test(Reg)) continue;
       
       // Ignore two-addr defs.
       if (MI->isRegTiedToUseOperand(i)) continue;
 
-      // FIXME: we should use a SubRegIterator that includes self (as above), so
-      // we don't have to repeat all this code for the reg itself.
-      DefIndices[Reg] = Count;
-      KillIndices[Reg] = ~0u;
-      assert(((KillIndices[Reg] == ~0u) !=
-              (DefIndices[Reg] == ~0u)) &&
-             "Kill and Def maps aren't consistent for Reg!");
-      KeepRegs.reset(Reg);
-      Classes[Reg] = nullptr;
-      RegRefs.erase(Reg);
-      // Repeat, for all subregs.
-      for (MCSubRegIterator SubRegs(Reg, TRI); SubRegs.isValid(); ++SubRegs) {
-        unsigned SubregReg = *SubRegs;
+      // For the reg itself and all subregs: update the def to current;
+      // reset the kill state, any restrictions, and references.
+      for (MCSubRegIterator SRI(Reg, TRI, true); SRI.isValid(); ++SRI) {
+        unsigned SubregReg = *SRI;
         DefIndices[SubregReg] = Count;
         KillIndices[SubregReg] = ~0u;
         KeepRegs.reset(SubregReg);
         Classes[SubregReg] = nullptr;
         RegRefs.erase(SubregReg);
       }
       // Conservatively mark super-registers as unusable.
       for (MCSuperRegIterator SR(Reg, TRI); SR.isValid(); ++SR)
for (unsigned i = 0, e = MI->getNumOperands(); i != e; ++i) {
     // class is consistent across all uses.
     if (!Classes[Reg] && NewRC)
       Classes[Reg] = NewRC;
     else if (!NewRC || Classes[Reg] != NewRC)
       Classes[Reg] = reinterpret_cast<TargetRegisterClass *>(-1);
 
     RegRefs.insert(std::make_pair(Reg, &MO));
 
-    // FIXME: we should use an MCRegAliasIterator that includes self so we don't
-    // have to repeat all this code for the reg itself.
-    
     // It wasn't previously live but now it is, this is a kill.
-    if (KillIndices[Reg] == ~0u) {
-      KillIndices[Reg] = Count;
-      DefIndices[Reg] = ~0u;
-          assert(((KillIndices[Reg] == ~0u) !=
-                  (DefIndices[Reg] == ~0u)) &&
-               "Kill and Def maps aren't consistent for Reg!");
-    }
-    // Repeat, for all aliases.
-    for (MCRegAliasIterator AI(Reg, TRI, false); AI.isValid(); ++AI) {
+    // Repeat for all aliases.
+    for (MCRegAliasIterator AI(Reg, TRI, true); AI.isValid(); ++AI) {
       unsigned AliasReg = *AI;
       if (KillIndices[AliasReg] == ~0u) {
         KillIndices[AliasReg] = Count;
         DefIndices[AliasReg] = ~0u;
       }
     }
   }
 }