[PATCH] D19675: [InstCombine][AVX2] Add support for simplifying AVX2 per-element shifts to native shifts

[David Majnemer via llvm-commits]

majnemer added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:339-354
@@ +338,18 @@
+    return nullptr;
+  case Intrinsic::x86_avx2_psrav_d:
+  case Intrinsic::x86_avx2_psrav_d_256:
+    LogicalShift = false;
+    ShiftLeft = false;
+    break;
+  case Intrinsic::x86_avx2_psrlv_d:
+  case Intrinsic::x86_avx2_psrlv_d_256:
+  case Intrinsic::x86_avx2_psrlv_q:
+  case Intrinsic::x86_avx2_psrlv_q_256:
+    LogicalShift = true;
+    ShiftLeft = false;
+    break;
+  case Intrinsic::x86_avx2_psllv_d:
+  case Intrinsic::x86_avx2_psllv_d_256:
+  case Intrinsic::x86_avx2_psllv_q:
+  case Intrinsic::x86_avx2_psllv_q_256:
+    LogicalShift = true;
----------------
RKSimon wrote:
> majnemer wrote:
> > Er, why do we even have these intrinsics?
> > They seem sorta pointless, it seems way better to get rid of these and replace their use in clang's headers with a generic shift which sanitizes the shift amount.
> What kind of sanitization can we add that doesn't raise the risk of adding extra code in there for unknown shift amounts? We can't just use a mask - it has to be a full CMP/MAX style test afaict.
That should be rather easy to match against in the DAG, no?

We do this sort of sanitization today for other intrinsics:
https://github.com/llvm-mirror/clang/blob/master/lib/Headers/avxintrin.h#L1329


Repository:
  rL LLVM

http://reviews.llvm.org/D19675