[PATCH] D115862: [LoongArch][test] (5/5) Add encoding and mnemonics tests

Lu Weining via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 20 19:59:12 PST 2021


SixWeining added a comment.

In D115862#3202374 <https://reviews.llvm.org/D115862#3202374>, @xen0n wrote:

> These encoding format names are atrocious to look at and use, especially the "non-standard" ones not mentioned in the manuals yet still have to be named. The names mostly start with a number so cannot be used directly as identifiers, and the irregular formats are named after instructions themselves -- what if some instructions are added later that happen to share the same encoding, like renaming the format is okay?
>
> For fixing these problems once and for good, please consider adopting the unambiguous notation defined by the loongarch-opcodes project <https://github.com/loongson-community/loongarch-opcodes>. While you may or may not like the idea of renaming a few instruction mnemonics, the instruction format naming is fixed for you, and I really hope the official manuals could follow suit and just ditch the `xRIxx` names.

@xen0n Thanks for your suggestion. We have read the notation defined in the repo you referenced. Yes, one name can exactly specifies one encoding. But there are a couple of issues left to adopt it in llvm. For example:

- immediate operand

Some immediate operands usually have specail meanings like the `msbd` and `lsbd` of the `bstrpick.d` instruction.  This instruction will be named with `DJUk6Um6` under the natation you proposed and thus lost the meaning of `msbd` and `lsbd` because we have to use names like `imm1` and `imm2` in instruction definition.

In current impl:

  // FmtBSTR_D
  // <opcode | msbd | lsbd | rj | rd>
  class FmtBSTR_D<bits<10> op, dag outs, dag ins, string asmstr,
                  list<dag> pattern = []> : LAInst<outs, ins, asmstr, pattern> {
    bits<6> msbd;
    bits<6> lsbd;
    bits<5> rj; 
    bits<5> rd; 
  
    let Inst{31-22} = op; 
    let Inst{21-16} = msbd;
    let Inst{15-10} = lsbd;
    let Inst{9-5} = rj; 
    let Inst{4-0} = rd; 
  }
  ...
  
  class ALU_BSTRD<bits<10> op, string opstr, Operand ImmOpnd>
      : FmtBSTR_D<op, (outs GPR:$rd), (ins GPR:$rj, ImmOpnd:$msb, ImmOpnd:$lsb),
                  !strconcat(opstr, "\t$rd, $rj, $msb, $lsb")>;
  
  ...
  
  def BSTRPICK_D : ALU_BSTRD<0b0000000011, "bstrpick.d", uimm6>;

Wth your proposal:

  // DJUk6Um6
  // <opcode | imm1 | imm2 | rj | rd>
  class DJUk6Um6<bits<10> op, dag outs, dag ins, string asmstr,
                 list<dag> pattern = []> : LAInst<outs, ins, asmstr, pattern> {
    bits<6> imm1; 
    bits<6> imm2;  
    bits<5> rj;                                              
    bits<5> rd;
      
    let Inst{31-22} = op;
    let Inst{21-16} = imm1;
    let Inst{15-10} = imm2;
    let Inst{9-5} = rj;
    let Inst{4-0} = rd;
  }
  
  ...
  
  def BSTRPICK_D : DJUk6Um6<0b0000000011, (outs GPR:$rd), (ins GPR:$rj, uimm6:$imm1, uimm6:$imm2), !strconcat(opstr, "\t$rd, $rj, $imm1, $imm2")>;



- Understanding cost

Inoder to understand what the name like `DJUk6Um6` means people may have to spend more time than the current impl.

But we do not totally disagree this proposal and let's wait more reviewers' comments about that. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115862



More information about the llvm-commits mailing list