[PATCH] D36395: [InstCombine] narrow rotate left/right patterns to eliminate zext/trunc (PR34046)

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D36395#834263, @efriedma wrote:

> I spent a bit of time playing with the generated for for various implementations of rotate for 16-bit values.  It looks like this transform actually makes things worse; e.g. on ARM the transformed version compiles to one more instruction.  Granted, that's probably not important; I think the optimal lowering for a 16-bit rotate on ARM actually involves lowering it to a 32-bit rotate and fixing up the result, and that's probably a transform which doesn't make sense until legalization.


I just looked at this, and I think it's a non-issue because the transform is guarded by shouldChangeType(). We shouldn't transform to i8/i16 for ARM/AArch64 because they don't list those types as legal in the datalayout:

$ ./clang -O1 rot16.c -S -o - -target aarch64 -emit-llvm
...
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"

$ ./clang -O1 rot16.c -S -o - -target x86_64 -emit-llvm
...
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"


https://reviews.llvm.org/D36395