[PATCH] D27220: [X86][SSE] Add support for target shuffle constant folding

[Zvi Rackover via Phabricator via llvm-commits]

zvi added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5073
+    auto *COp = Cst->getAggregateElement(i);
+    if (!COp || (!isa<UndefValue>(COp) && !isa<ConstantInt>(COp) &&
+                 !isa<ConstantFP>(COp)))
----------------
Consider reducing the negations to something like:
(!(COp && (isa<UndefValue>(COp) || isa<ConstantInt>(COp) ||
                   isa<ConstantFP>(COp))))


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:5084
+    APInt Bits;
+    if (isa<ConstantInt>(COp))
+      Bits = cast<ConstantInt>(COp)->getValue();
----------------
Consider using dyn_cast for these two if's


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26414
+
+  // Only fold if at least one of the constants is only used once or
+  // the combined shuffle has included a variable mask shuffle.
----------------
I'm asking for educational reasons and not because I don't agree with this:
Why is if at least one constant is used once (and not all or none) is the combine profitable?


Repository:
  rL LLVM

https://reviews.llvm.org/D27220