[PATCH] D119817: [LoongArch] Fix several instruction definition errors in initial pathes

[WÁNG Xuěruì via Phabricator via llvm-commits]

xen0n added a comment.

Side comment: now you see why I insisted on auto-generating instruction definitions, so this kind of typo wouldn't happen in the first place ;-)



================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrFormats.td:263
+  bits<5> msbw;
+  bits<5> lsbw;
   bits<5> rj;
----------------
Okay so this is being changed to exactly match manual syntax. While I believe the semantics is not affected at all, it's okay nevertheless, because at this point everything else matches manual description. (However ugly/inconsistent that is; but again, changing these is outside the scope of the initial bring-up.)


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:146
 class LLBase<bits<8> op, string opstr>
-    : Fmt2RI14<op, (outs GPR:$rd), (ins GPR:$rj, simm14:$imm14),
+    : Fmt2RI14<op, (outs GPR:$rd), (ins GPR:$rj, simm14_lsl2:$imm14),
                !strconcat(opstr, "\t$rd, $rj, $imm14")>;
----------------
Fair enough, the original description is clearly wrong.


================
Comment at: llvm/lib/Target/LoongArch/LoongArchInstrInfo.td:210-216
+let Constraints = "$rd = $dst" in
+def BSTRINS_W  : FmtBSTR_W<0b000000000110, (outs GPR:$dst),
+                           (ins GPR:$rd, GPR:$rj, uimm5:$msbw, uimm5:$lsbw),
+                           "bstrins.w\t$rd, $rj, $msbw, $lsbw">;
+def BSTRPICK_W : FmtBSTR_W<0b000000000111, (outs GPR:$rd),
+                           (ins GPR:$rj, uimm5:$msbw, uimm5:$lsbw),
+                           "bstrpick.w\t$rd, $rj, $msbw, $lsbw">;
----------------
So every other line in this section are simple references to already-nicely-wrapped templates, except these two? I think keeping the previous style of writing would be nicer.

Also, is the constraint meant to cover the first def only? I guess you may want to cover both, so this may need revision too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119817