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

[Alex Bradbury via Phabricator via llvm-commits]

asb accepted this revision.
asb added a comment.
This revision is now accepted and ready to land.

Thanks, looks good to me.



================
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))
----------------
kparzysz wrote:
> 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.
Yes,  I figured that was why. I did wonder if LLVM's codegen would be smart enough to optimise the repeated traversal. It's not an important point one way or the other anyway.


Repository:
  rL LLVM

https://reviews.llvm.org/D31937