[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:362
@@ +361,3 @@
+  // Simplify if all shift amounts are constant/undef.
+  Constant *CShift = dyn_cast<Constant>(II.getArgOperand(1));
+  if (!CShift)
----------------
`auto *CShift`

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:377-384
@@ +376,10 @@
+  for (int I = 0; I < NumElts; ++I) {
+    Constant *COp = CShift->getAggregateElement(I);
+    if (!COp || (!isa<UndefValue>(COp) && !isa<ConstantInt>(COp)))
+      return nullptr;
+
+    if (isa<UndefValue>(COp)) {
+      ShiftAmts.push_back(-1);
+      continue;
+    }
+
----------------
You can simplify this by handling the `UndefValue` case upfront:

```
    if (isa<UndefValue>(COp)) {
      ShiftAmts.push_back(-1);
      continue;
    }

    auto *COp = dyn_cast_or_null<ConstantInt>(CShift->getAggregateElement(I));
    if (!COp)
      return nullptr;
```

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:393
@@ +392,3 @@
+      AnyOutOfRange = LogicalShift;
+      ShiftVal = APInt(64, LogicalShift ? BitWidth : BitWidth - 1);
+    }
----------------
Why not just `ShiftAmts.push_back(LogicalShift ? BitWidth : BitWidth - 1)`?

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:402
@@ +401,3 @@
+  auto OutOfRange = [&](int Idx) { return (Idx < 0) || (BitWidth <= Idx); };
+  if (std::all_of(ShiftAmts.begin(), ShiftAmts.end(), OutOfRange)) {
+    SmallVector<Constant *, 8> ConstantVec;
----------------
Can you `llvm::all_of`?

================
Comment at: lib/Transforms/InstCombine/InstCombineCalls.cpp:404-411
@@ +403,10 @@
+    SmallVector<Constant *, 8> ConstantVec;
+    for (int Idx : ShiftAmts)
+      if (Idx < 0) {
+        ConstantVec.push_back(UndefValue::get(SVT));
+      } else {
+        assert(LogicalShift && "Logical shift expected");
+        ConstantVec.push_back(ConstantInt::getNullValue(SVT));
+      }
+    return ConstantVector::get(ConstantVec);
+  }
----------------
Braces.


Repository:
  rL LLVM

http://reviews.llvm.org/D19675