[PATCH] D120476: [LoongArch] Add basic support to AsmParser

Lu Weining via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 3 21:23:12 PST 2022


SixWeining added inline comments.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:65
+
+def simm12 : Operand<GRLenVT>, ImmLeaf<GRLenVT, [{return isInt<12>(Imm);}]> {
+  let ParserMatchClass = SImmAsmOperand<12>;
----------------
xen0n wrote:
> I may have missed this in the previous patches, but why does only this kind of imm get the `ImmLeaf` treatment? I briefly looked at the other targets and they seem to mark most of their commonly used immediate kinds as such.
The `ImmLeaf` is used in instruction selection stage to match the immediate. Yes, all immediate kinds that could be used in instruction selection should be defined with `ImmLeaf`. They will added when we implement the `codegen`.

The reason why only `simm12` is defined with `ImmLeaf` is that in line 491 we defined a few instruction selection `Pat`s: 
```
...
def : PatGprImm<add, ADDI_W, simm12>;
...
```

These `Pat`s here are just to demonstrate layout of this `.td` file and the way to define `Pat`. Of course, more `Pat`s will be defined later to support `codegen`.

Sorry for the confusion. I hope I make it clear now.


================
Comment at: llvm/lib/Target/LoongArch/MCTargetDesc/LoongArchMCTargetDesc.cpp:122
 
     // Register the MCInstPrinter.
     TargetRegistry::RegisterMCInstPrinter(*T, createLoongArchMCInstPrinter);
----------------
xen0n wrote:
> Considering that every line in this function is self-documenting, do we really need any of them? At least with the addition below, *this* line becomes inaccurate...
I agree with you. Let me delete them in a seperate patch. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120476



More information about the llvm-commits mailing list