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

[Fabian Giesen via Phabricator via llvm-commits]

fabiang added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5695
+    SDValue ShY = DAG.getNode(ISD::SRL, sdl, VT, Y, IsFSHL ? InvShAmt : ShAmt);
+    setValue(&I, DAG.getNode(ISD::OR, sdl, VT, ShX, ShY));
+    return nullptr;
----------------
Wait, I don't think this lowering works when Z=0 (mod BitWidth) when BitWidth is the width of the register; not on shift-amounts-modulo-register-width architectures anyway. (It should work on PPC.)

e.g. for 32-bit fshl and with shift widths taken modulo 32, we get (X << 0) | (Y >> 32) == (X >> 0) | (Y >> 0) == X | Y, not the expected X.

For compile-time constant Z the Z=0 case can be handled, and when X=Y (i.e. rotate) it turns out to be harmless, but for X!=Y and variable Z that happens to be congruent to 0 I think this is trouble.


https://reviews.llvm.org/D49242