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

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz added inline comments.


================
Comment at: include/llvm/Target/TargetRegisterInfo.h:320
+  /// Return true if the given TargetRegisterClass has the ValueType T.
+  bool hasType(const TargetRegisterClass &RC, MVT T) const {
+    for (int i = 0; RC.VTs[i] != MVT::Other; ++i)
----------------
MatzeB wrote:
> kparzysz wrote:
> > MatzeB wrote:
> > > The name TargetRegisterInfo::hasType feels odd as this isn't a general property of TargetRegisterInfo. Maybe rename to `regclassHasType()`?
> > > 
> > > Similar with the next two functions (`regclassTypes_begin()` maybe?).
> > How about `isTypeLegalForClass` and `regclassvt_begin`, `regclassvt_end`?
> I'd be fine with them independently. Though maybe there is a way to make the look more similar (as they are all about the same concept). `isTypeLegalForClass()`, `legalClassTypesBegin()`, `legalClassTypesEnd()`?
>From what I've seen elsewhere, the convention for iterators is "lowercase with _begin/_end". Do legalclasstypes_begin and legalclasstypes_end look ok to you?


Repository:
  rL LLVM

https://reviews.llvm.org/D31937