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

[Matthias Braun via Phabricator via llvm-commits]

MatzeB 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)
----------------
kparzysz wrote:
> 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?
Guess it's llvm naming conventions clashing with STL ones, looks good.


Repository:
  rL LLVM

https://reviews.llvm.org/D31937