[PATCH] D63205: Virtualize TargetInstrInfo::getRegClass()

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In D63205#1546855 <https://reviews.llvm.org/D63205#1546855>, @rampitec wrote:

> In D63205#1546727 <https://reviews.llvm.org/D63205#1546727>, @bogner wrote:
>
> > Maybe I'm missing something, but the override implemented in D63204 <https://reviews.llvm.org/D63204> looks like a subset of the default `TII::getRegClass` that falls through to `TRI::getRegClass`, just like the default does. Is this change actually necessary?
>
>
> It will fallback to TRI::getRegClass(), but to the base class of it through the pointer to TRI in the InstrEmitter, so call goes to the default implementation. Either TII::getRegClass() needs to be virtual or TRI::getRegClass().
>  I have preferred to override in TII because it is called less frequently than from TRI.


I.e. without override it ends up in this implementation of TargetRegisterInfo::getRegClass(): https://llvm.org/doxygen/TargetRegisterInfo_8h_source.html#l00700
With the override it ends up here: http://llvm.org/doxygen/SIRegisterInfo_8cpp_source.html#l01727
Since it is only needed at selection the override is in the TII to make it lighter. An override in TRI would equally work, but will unnecessary result in more virtual calls than needed.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63205/new/

https://reviews.llvm.org/D63205