[PATCH] D117592: [ARM][AArch64] Introduce qrdmlah and qrdmlsh intrinsics

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

In D117592#3254003 <https://reviews.llvm.org/D117592#3254003>, @DavidSpickett wrote:

> Thanks for working on this!
>
> qrdmilh -> qrdmulh in the commit message.
>
> I'm out of my depth math wise here so can't approve overall but perhaps you can clarify the aim here.
>
>> Since it's introduction, the qrdmlah has been represented as a qrdmilh and a sadd_sat. This doesn't produce the same result for all input values though. This patch fixes that by introducing a qrdmlah (and qrdmlsh) intrinsic specifically for the vqrdmlah and sqrdmlah instructions. The old test cases will now produce a qrdmulh and sqadd, as expected.
>
>
>
> - We had `qrdmlah` which output `qrdmulh` then `sadd_sat`. This didn't behave as expected for one set of inputs.
> - You've introduced an intrinsic for `qrdmlah`. So now it'll output the one `vqrdmlah` instruction. This behaves as expected for all inputs.
> - The existing test cases will now (have always?) output `qrdmulh` and `sqadd`.
>
> That last bit I don't understand. Wouldn't those test cases output the new single instruction? I think I am misunderstanding which cases these are and what they test.

Yeah - The vqrdmlah ACLE intrinsic will produce a single vqrdmlah (or sqrdmlah). That should all be fine. The test cases with sadd.sat + arm.neon.vqrdmulh will now produce two instructions (as you would expect).


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

https://reviews.llvm.org/D117592