[PATCH] D49242: [Intrinsics] define funnel shift IR intrinsics + DAG builder support

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: docs/LangRef.rst:11924
+
+      %r = call i8 @llvm.fshl.i8(i8 %x, i8 %y, i8 %z)  ; %r = i8: extract((concat(%x, %y) << %z), 0, 7)
+      %r = call i8 @llvm.fshl.i8(i8 255, i8 0, i8 15)  ; %r = i8: 128 (0b10000000)
----------------
fabiang wrote:
> I think this should be extract((contact(%x, %y) << %z), 8, 15) since it grabs the more significant half of the shifted double-width value.
Do we have any precedence/documentation for bit numbering of LLVM IR values?
IIUC, you're seeing this:
MSB:15 -> LSB:0

but I was referencing this:
MSB:0 -> LSB 15

Not sure if this is relevant, but we count vector elements from 0-on-the-left:
"The elements of the two input vectors are numbered from left to right across both of the vectors."

Either way, I will add an explanatory comment.


================
Comment at: docs/LangRef.rst:11970
+
+      %r = call i8 @llvm.fshr.i8(i8 %x, i8 %y, i8 %z)  ; %r = i8: extract((concat(%x, %y) >> %z), 8, 15)
+      %r = call i8 @llvm.fshr.i8(i8 255, i8 0, i8 15)  ; %r = i8: 128 (0b00000001)
----------------
fabiang wrote:
> Conversely, this one should extract bits 0 through 7 not 8 through 15.
Same problem as above - we need to clarify the bit numbering that we're using.


================
Comment at: docs/LangRef.rst:11971
+      %r = call i8 @llvm.fshr.i8(i8 %x, i8 %y, i8 %z)  ; %r = i8: extract((concat(%x, %y) >> %z), 8, 15)
+      %r = call i8 @llvm.fshr.i8(i8 255, i8 0, i8 15)  ; %r = i8: 128 (0b00000001)
+      %r = call i8 @llvm.fshr.i8(i8 15, i8 15, i8 11)  ; %r = i8: 225 (0b11100001)
----------------
fabiang wrote:
> I think this example needs the first and second arguments swapped. (Alternatively, the result should be 0xfe = 0b11111110)
Yes - good catch.


https://reviews.llvm.org/D49242