[PATCH] D115861: [LoongArch 4/6] Add basic tablegen infra for LoongArch

[Renato Golin via Phabricator via llvm-commits]

rengolin added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrFormats.td:43
+// <opcode | rj | rd>
+class Fmt2R<bits<22> op, dag outs, dag ins, string asmstr,
+            list<dag> pattern = []> : LAInst<outs, ins, asmstr, pattern> {
----------------
SixWeining wrote:
> xen0n wrote:
> > As discussed in D115862, people should reach consensus on instruction format naming before continuing with coding.
> Right. But this patch is parent of D115862 but not child.
> 
> Do other people have any comments for the instruction format naming? @rengolin @MaskRay 
I don't actually have a strong opinion on this...

My personal view is that this is at such a low level that (almost) only people that work on the target will have to deal with, so what constitutes "easy to read and understand" depends on the target's own context.

For example abbreviations of target-specific terms may not make much sense to other people but they're very clear to that target engineers.

Having said that, I do appreciate @xen0n's point of having a consistent format. This is more than just "what feels right" and is more about "how easy it is for outside people understand the code (by reading an external doc, for ex.)" as well as "what scales better and is more maintainable".

The argument that this is a new target and we ought to do better is also a good one. But at the same time, if there are different back-ends to other tools and only this one is different, that'd add maintenance cost to the target's community.

And while I agree that setting a good precedent is worthwhile, there could be resistance from other compilers/emulators to refactor their code without clear benefits, so the difference may end up permanent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115861