[PATCH] D137934: [InstCombine] Fold extractelt with select of constants

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

This seems close to complete now. Please pre-commit the tests with baseline CHECKs as an NFC patch (no pre-commit review needed). That way, we'll just show test diffs in this patch.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:401-403
+  // We don't want to produce a select <n x i1>, c1, c2
+  // instruction when constant folding a extractelt
+  // into a select:
----------------
Omit this description of a negative pattern. The formula under this line describes what we expect to happen. 
One minor adjustment - `c` generally refers to a constant value in this code, so use some other letter for the condition:
  // extractelt (select %x, <vec1>, <vec2>), %const ->
  // select %x, <vec1>[%const], <vec2>[%const]


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:600
   }
+
   return nullptr;
----------------
Remove whitespace changes, so the patch is minimal. 
You could do a more extensive formatting cleanup as an NFC patch if you want.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1072
+  if (auto *EI = dyn_cast<ExtractElementInst>(&I))
+      return Builder.CreateExtractElement(SO, EI->getIndexOperand());
+
----------------
Tab settings are off. Adjust your editor to conform with existing code or use `clang-format`.


================
Comment at: llvm/lib/Transforms/InstCombine/InstructionCombining.cpp:1165
+  if (auto *EI = dyn_cast<ExtractElementInst>(&Op))
+      if (auto *C = dyn_cast<Constant>(EI->getIndexOperand()); !C)
+          return nullptr;
----------------
Avoid `dyn_cast` if we're not using the captured value:
  if (!isa<Constant>(EI->getIndexOperand()))


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137934/new/

https://reviews.llvm.org/D137934