[PATCH] D15250: Detecte vector reduction operations just before instruction selection.

Cong Hou via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 19 12:56:36 PST 2016


congh marked 8 inline comments as done.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2367
@@ +2366,3 @@
+
+      if (Inst->getOpcode() == OpCode || isa<PHINode>(U)) {
+        UsersToVisit.push_back(U);
----------------
hfinkel wrote:
> hfinkel wrote:
> > hfinkel wrote:
> > > We also need to check the fast-math flags on fadd/fmul here.
> > We can allow selects here for the same reason we can allow phis, right? If so, we should.
> (Specifically, I mean selects with a scalar condition; I assume that selects with a vector condition would not work here).
When I wrote this pattern recognition, I am more concerned about the compiler vectorized code, which normally won't have selects. We detect phi because it can appear in the beginning of the loop, which is not the case of selects. Do you think if we should make this pattern recognition complicated enough to catch all cases that are manually composed?

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2375
@@ +2374,3 @@
+        // ElemNumToReduce / 2 elements in another vector.
+
+        if (!isa<UndefValue>(U->getOperand(1)))
----------------
hfinkel wrote:
> Do we need to check that we don't have ElemNumToReduce == 1 here?
I am not sure if it could happen, but I added such a check just in case.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2398
@@ +2397,3 @@
+          UsersToVisit.push_back(U2);
+          ElemNumToReduce /= 2;
+        } else
----------------
hfinkel wrote:
> When  ElemNumToReduce == 1 here, do we need to check that the only user is an ExtractElementInst? I'm concerned that it could be a another reduction operation before the extract.
I think even there is another reduction operation just before extract, it is still OK. We only care about if all values in vector are reduced to one element and this is still the case.


http://reviews.llvm.org/D15250





More information about the llvm-commits mailing list