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

[Cullen Rhodes via Phabricator via llvm-commits]

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
----------------
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!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62130