[PATCH] [DagCombiner] Allow shuffles to merge through bitcasts

Andrea Di Biagio Andrea_DiBiagio at sn.scee.net
Thu Mar 5 04:30:19 PST 2015


Hi Simon,

Overall the patch looks good to me. I suggest that you move this new logic into a separate function. In my opinion, It would make the code in visitVECTOR_SHUFFLE a bit more readable. I also made a couple of minor comments (see below).

Thanks!
-Andrea


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11883
@@ +11882,3 @@
+  // back to their original types.
+  if (N0.getOpcode() == ISD::BITCAST && N->isOnlyUserOf(N0.getNode()) &&
+      N1.getOpcode() == ISD::UNDEF && Level < AfterLegalizeVectorOps &&
----------------
In this context, we know that N0 is used by N, so writing
'N->isOnlyUserOf(N0.getNode())' is equivalent to writing 'N0.hasOneUse()'. If there is one use, then it can only be N.

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11899-11907
@@ +11898,11 @@
+
+      SmallVector<int, 8> NewMask;
+      for (int M : Mask)
+        for (int i = 0; i != Scale; ++i) {
+          if (M < 0) {
+            NewMask.push_back(-1);
+            continue;
+          }
+          NewMask.push_back(Scale * M + i);
+        }
+      return NewMask;
----------------
A very minor nit:
You can probably explicitly initialize NewMask like this:
SmallVector<int, 8> NewMask(Scale, -1);

This will allow you to simplify the loop using operator[]. So, `NewMask.push_back(Scale * M + i)` would become `NewMask[i] = Scale * M + i`.
Also, if you do that, then you would not need to always 'push_back(-1)' if M is less than 0.

http://reviews.llvm.org/D7939

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list