[PATCH] D152311: [RISCV] Merge RISCVMCInstLower.cpp into RISCVAsmPrinter.cpp.

[Alex Bradbury via Phabricator via llvm-commits]

asb added a comment.

In D152311#4402018 <https://reviews.llvm.org/D152311#4402018>, @craig.topper wrote:

> In D152311#4401941 <https://reviews.llvm.org/D152311#4401941>, @asb wrote:
>
>> This is definitely something I just copied from other backends and I like the idea of merging it. There is some value in uniformity with other backends though, so if other targets were dead set against ever considering something similar then perhaps we'd consider if it's better to stay matching everyone else.
>
> I think there's already inconsistency in the structure of the files. I surveyed some of the targets.
>
> X86 has a X86MCInstLower class that is instantiated on each call to X86AsmPrinter::emitInstruction. X86AsmPrinter::emitInstruction is implemented in X86MCInstLower.cpp
>
> RISC-V does not have a MCInstLower class.
>
> AArch64 has a AArch64MCInstLower object that is a member of AArch64AsmPrinter so its only instantiated once.
>
> ARM, Hexagon, PowerPC, and SystemZ seem similar to RISC-V.
>
> SystemZ has a SystemZMCInstLower instatiated from SystemZAsmPrinter::emitInstruction. SystemZAsmPrinter::emitInstruction is in SystemZAsmPrinter.cpp
>
> AMDGPU has a AMDGPUMCInstLower instantiated from AMDGPUAsmPrinter::emitInstruction. AMDGPUAsmPrinter::emitInstruction lives in AMDGPUMCInstLower.cpp.
>
> Mips has a MipsMCInstLower member of MipsAsmPrinter so it's only instantiated once.



In D152311#4402018 <https://reviews.llvm.org/D152311#4402018>, @craig.topper wrote:

> In D152311#4401941 <https://reviews.llvm.org/D152311#4401941>, @asb wrote:
>
>> This is definitely something I just copied from other backends and I like the idea of merging it. There is some value in uniformity with other backends though, so if other targets were dead set against ever considering something similar then perhaps we'd consider if it's better to stay matching everyone else.
>
> I think there's already inconsistency in the structure of the files. I surveyed some of the targets.
>
> X86 has a X86MCInstLower class that is instantiated on each call to X86AsmPrinter::emitInstruction. X86AsmPrinter::emitInstruction is implemented in X86MCInstLower.cpp
>
> RISC-V does not have a MCInstLower class.
>
> AArch64 has a AArch64MCInstLower object that is a member of AArch64AsmPrinter so its only instantiated once.
>
> ARM, Hexagon, PowerPC, and SystemZ seem similar to RISC-V.
>
> SystemZ has a SystemZMCInstLower instatiated from SystemZAsmPrinter::emitInstruction. SystemZAsmPrinter::emitInstruction is in SystemZAsmPrinter.cpp
>
> AMDGPU has a AMDGPUMCInstLower instantiated from AMDGPUAsmPrinter::emitInstruction. AMDGPUAsmPrinter::emitInstruction lives in AMDGPUMCInstLower.cpp.
>
> Mips has a MipsMCInstLower member of MipsAsmPrinter so it's only instantiated once.

Thanks - I upgrade my view from supportive to _very_ supportive!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152311