Virtualize TargetInstrInfo::getRegClass()

[rampitec]

Summary

AMDGPU target needs to override getRegClass() used during
instruction selection. We now may have either 32 or 64 bit
conditional registers used in the same instructions. For
that purpose special SReg_1 register class is created which
is dynamically resolved to either SReg_64 or SGPR_32 depending
on the subtarget attributes.

See AMDGPU specific change D63204 for the usage.

[rampitec]

Ping

[bogner]

Maybe I'm missing something, but the override implemented in 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?

[rampitec]

In D63205#1546727, @bogner wrote:

Maybe I'm missing something, but the override implemented in 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.

[rampitec]

In D63205#1546855, @rampitec wrote:

In D63205#1546727, @bogner wrote:

Maybe I'm missing something, but the override implemented in 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.

[msearles]

LGTM

include/llvm/CodeGen/TargetInstrInfo.h

public:
   virtual ~TargetInstrInfo();
 
   static bool isGenericOpcode(unsigned Opc) {
     return Opc <= TargetOpcode::GENERIC_OP_END;
   }
 
   /// Given a machine instruction descriptor, returns the register
   /// class constraint for OpNum, or NULL.
+  virtual
   const TargetRegisterClass *getRegClass(const MCInstrDesc &MCID, unsigned OpNum,
                                          const TargetRegisterInfo *TRI,
                                          const MachineFunction &MF) const;
 
   /// Return true if the instruction is trivially rematerializable, meaning it
   /// has no side effects and requires no operands that aren't always available.
   /// This means the only allowed uses are constants and unallocatable physical
   /// registers so that the instructions result is independent of the place