[PATCH] Remove redundant canonicalization rules from method DAGCombiner::visitVECTOR_SHUFFLE.

Andrea Di Biagio andrea.dibiagio at gmail.com
Tue Aug 12 11:07:40 PDT 2014


Hi,

Target independent shuffle dag nodes are only created by the backend
using method 'SelectionDAG::getVectorShuffle'. Currently, ff we try to
use the overloaded method 'getNode' that takes as input three
SDValues, the compiler would eventually hit a
`llvm_unreachable("should use getVectorShuffle constructor!")`. That
suggests that the only possible/safe way to create ISD shuffles is
through method 'getVectorShuffle'.

Method 'SelectionDAG::getVectorShuffle' knows how to perform some
basic simplifications in order to always emit a "canonical" dag node.
For example, it knows how to canonicalize a shuffle according to
rules:
  // Canonicalize shuffle undef, undef -> undef
  // Canonicalize shuffle v, v -> v, undef
  // Canonicalize shuffle undef, v -> shuffle v, undef

Those same rules are currently duplicated within method
'DAGCombiner::visitVECTOR_SHUFFLE'. If my previous reasoning is
correct, then those rules are "dead" (i.e. we never trigger those
rules from the DAGCombiner).
Therefore, it should be safe to remove the redundant canonicalization
rules from visitVECTOR_SHUFFLE.

This patch simplifies the code in method
'DAGCombiner::visitVECTOR_SHUFFLE' removing the above-mentioned "dead"
canonicalization rules.
No functional change intended.

Please let me know if this is ok to submit.

Thanks,
Andrea
-------------- next part --------------
Index: lib/CodeGen/SelectionDAG/DAGCombiner.cpp
===================================================================
--- lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(revision 215459)
+++ lib/CodeGen/SelectionDAG/DAGCombiner.cpp	(working copy)
@@ -10660,57 +10660,8 @@
 
   assert(N0.getValueType() == VT && "Vector shuffle must be normalized in DAG");
 
-  // Canonicalize shuffle undef, undef -> undef
-  if (N0.getOpcode() == ISD::UNDEF && N1.getOpcode() == ISD::UNDEF)
-    return DAG.getUNDEF(VT);
-
   ShuffleVectorSDNode *SVN = cast<ShuffleVectorSDNode>(N);
 
-  // Canonicalize shuffle v, v -> v, undef
-  if (N0 == N1) {
-    SmallVector<int, 8> NewMask;
-    for (unsigned i = 0; i != NumElts; ++i) {
-      int Idx = SVN->getMaskElt(i);
-      if (Idx >= (int)NumElts) Idx -= NumElts;
-      NewMask.push_back(Idx);
-    }
-    return DAG.getVectorShuffle(VT, SDLoc(N), N0, DAG.getUNDEF(VT),
-                                &NewMask[0]);
-  }
-
-  // Canonicalize shuffle undef, v -> v, undef.  Commute the shuffle mask.
-  if (N0.getOpcode() == ISD::UNDEF) {
-    SmallVector<int, 8> NewMask;
-    for (unsigned i = 0; i != NumElts; ++i) {
-      int Idx = SVN->getMaskElt(i);
-      if (Idx >= 0) {
-        if (Idx >= (int)NumElts)
-          Idx -= NumElts;
-        else
-          Idx = -1; // remove reference to lhs
-      }
-      NewMask.push_back(Idx);
-    }
-    return DAG.getVectorShuffle(VT, SDLoc(N), N1, DAG.getUNDEF(VT),
-                                &NewMask[0]);
-  }
-
-  // Remove references to rhs if it is undef
-  if (N1.getOpcode() == ISD::UNDEF) {
-    bool Changed = false;
-    SmallVector<int, 8> NewMask;
-    for (unsigned i = 0; i != NumElts; ++i) {
-      int Idx = SVN->getMaskElt(i);
-      if (Idx >= (int)NumElts) {
-        Idx = -1;
-        Changed = true;
-      }
-      NewMask.push_back(Idx);
-    }
-    if (Changed)
-      return DAG.getVectorShuffle(VT, SDLoc(N), N0, N1, &NewMask[0]);
-  }
-
   // If it is a splat, check if the argument vector is another splat or a
   // build_vector with all scalar elements the same.
   if (SVN->isSplat() && SVN->getSplatIndex() < (int)NumElts) {


More information about the llvm-commits mailing list