[PATCH] D62130: [AArch64][SVE2] Asm: add saturating add/sub instructions

[Cullen Rhodes via Phabricator via llvm-commits]

c-rhodes marked an inline comment as done.
c-rhodes added inline comments.


================
Comment at: test/MC/AArch64/SVE2/sqadd.s:12
+// CHECK-INST: sqadd z0.b, p0/m, z0.b, z1.b
+// CHECK-ENCODING: [0x20,0x80,0x18,0x44]
+// CHECK-ERROR: instruction requires: sve2
----------------
c-rhodes wrote:
> chill wrote:
> > The encoding does not look right.
> > 
> > In  https://reviews.llvm.org/D62000 there's this bit:
> > 
> > ```class sve2_int_arith_pred<bits<2> sz, bits<6> opc, string asm,
> >                           ZPRRegOp zprty>
> > ...
> >   let Inst{21-20} = 0b01;
> >   let Inst{20-16} = opc{5-1};
> > ...
> > ```
> > 
> > which has overlapping bit 20.
> Good spot, bit 20 shouldn't overlap there, that should be:
> 
> ```
>   let Inst{21}    = 0b0;
>   let Inst{20-16} = opc{5-1};
> ```
> I believe the encodings are still correct however as bit 20 is defined by the `opc` bits and this overwrites the earlier error of setting bit 20 to 0. I've checked all the encodings for instructions defined by `sve2_int_arith_pred` and compared against other implementations and can't see any issues.
> 
> I'll create patch for the above to address the overlapping bit 20. Thanks for spotting this!
Removed overlapping bit in: https://reviews.llvm.org/D62292


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62130