[PATCH] D31937: Move value type list from TargetRegisterClass to TargetRegisterInfo

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz marked 2 inline comments as done.
kparzysz added inline comments.


================
Comment at: lib/CodeGen/TargetLoweringBase.cpp:1189
+                                   const TargetRegisterClass &RC) const {
+  for (auto I = TRI->valuetypes_begin(RC); *I != MVT::Other; ++I)
     if (isTypeLegal(*I))
----------------
asb wrote:
> Even though it's slightly less typing, I can't help but feel that checking for MVT::Other is peeking under the iterator abstraction unnecessarily.
The reason I did it was that the end iterator traverses the whole list, also unnecessarily.  We could have a separate class for the VT iterator, and implement something like "isValid" in it, but
1. The traversal only happens in a very few places, and it's not something that's expected to change, and
2. The ending with MVT::Other is specific to the VT list for a register class, and not any other list of value types.
With that in mind, I'm not sure that it's worth it.


Repository:
  rL LLVM

https://reviews.llvm.org/D31937