[PATCH] D46009: [AArch64] Custom Lower MULLH{S, U} for v16i8, v8i16, and v4i32

Adhemerval Zanella via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 2 10:20:00 PDT 2018


zatrazz added a comment.

In https://reviews.llvm.org/D46009#1083782, @SjoerdMeijer wrote:

> Sorry, I needed to get up to speed here, also with this hacker's delight trick. 
>  I think it would be good if you add some comments to LowerMULH what we are trying to do here.


Ack.

> I think I have some more comments, mainly about the tests:
> 
> - you're not checking the magic number (e.g. in the first test "movi  v1.16b, #57"), but I think that is quite relevant here.

Right, but the change itself is to correct lower the multiply high part of the divide by constant optimization. I have adjusted the next patch version to match the expected constant as well.

> - you're only using constant vectors with value 9. I think it would be good to vary here.

Ack.

> - I think you should also define the operands of the smull2 and smull instructions with regexps, and use them.

Ack.

> - nit: some functions still have inconsistent names, e.g. function "umul8xi16" tests "udiv <16 x i8>"

Ack, I have changed to div*.


https://reviews.llvm.org/D46009





More information about the llvm-commits mailing list