[PATCH] D101538: [GlobalISel][IRTranslator] Make translate() methods virtual.

[Aleksandr Bezzubikov via Phabricator via llvm-commits]

zuban32 added a comment.

In D101538#2753323 <https://reviews.llvm.org/D101538#2753323>, @foad wrote:

> In D101538#2751818 <https://reviews.llvm.org/D101538#2751818>, @aemerson wrote:
>
>> In D101538#2750853 <https://reviews.llvm.org/D101538#2750853>, @zuban32 wrote:
>>
>>> In D101538#2745606 <https://reviews.llvm.org/D101538#2745606>, @aemerson wrote:
>>>
>>>> Is this really necessary? Our preference is to use custom combines/lowering passes, or custom legalization, to do this.
>>>
>>> Well, our usecase might not be that canonical, let me briefly explain. For example, translating a bitcast - default IRTranslator implementation turns it into a COPY when LLTs of src and dst are the same. In our target (SPIR-V) LLTs do not matter that much, the typeinfo is represented with some pseudo instructions, so we run into this situation quite often. And losing a bitcast in the translator doesn't work for us as it'd obviously be quite difficult to recover it.
>>
>> I see. Is this a widespread issue with a lot of the translation functions or just bitcasts? If it's just bitcast you could just make that virtual so we don't have to pay the virtual function call overhead every translate. If not, then I'm ok with doing this.
>
> How about doing it with https://en.wikipedia.org/wiki/Curiously_recurring_template_pattern#Static_polymorphism to avoid the virtual function call overhead?

@foad hmm, with a naive approach I suppose we gonna run into the multiple inheritance problem. Let's assume we've created a base class as following:

  template<typename T>
  class BaseIRTranslator {
  protected:
    bool translate(const Instruction &Inst) {
      return static_cast<T *>(this)->translateImpl(Inst);
    }
    bool translate(const Constant &C, Register Reg) {
      return static_cast<T *>(this)->translateImpl(C, Reg);
    }
  };

and then renamed old IRTranslator::translate methods to translateImpl. Then we have a target which wants to override them, but this new TargetIRTranslator would still have to be inherited from the original IRTranslator too as we want to use most of its functionality, so we end up with something like

  class TargetIRTranslator : public IRTranslator, BaseIRTranslator<TargetIRTranslator> {
  protected:
    bool translateImpl(const Instruction &Inst);
    bool translateImpl(const Constant &C, Register Reg);
  }

and translate() methods are defined twice. We used to have virtual inheritance for such kind of problems, but in this case it kills the whole idea of CRTP. Pls correct if I'm wrong


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101538