[PATCH] D11121: Optimization for Gather/Scatter with uniform base

[Elena Demikhovsky]

delena added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3084
@@ -3083,2 +3083,3 @@
+// Get a unform base for the Gather/Scatter input.
 // Gather/scatter receive a vector of pointers.
 // This vector of pointers may be represented as a base pointer + vector of 
----------------
ab wrote:
> receive -> take, perhaps?
Vector of pointers is the first argument of Gather/Scatter

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3116-3117
@@ +3115,4 @@
+    ShuffleVectorInst *ShuffleInst = dyn_cast<ShuffleVectorInst>(GepBasePtr);
+    if (!ShuffleInst || !ShuffleInst->getMask()->isNullValue() ||
+        !isa<InsertElementInst>(ShuffleInst->getOperand(0)))
+      return false;
----------------
ab wrote:
> Should we check that the insertelement index is 0?  (or even generalize and check that the broadcast and insertelement have the same index, though that sounds unnecessary)
I submitted another patch with getSplatValue() as vector utility and planned to use it here, but it is stuck on review.
http://reviews.llvm.org/D11124


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3140
@@ +3139,3 @@
+  if (SExtInst* Sext = dyn_cast<SExtInst>(IndexVal)) {
+    IndexVal = Sext->getOperand(0);
+    if (SDB->findValue(IndexVal))
----------------
ab wrote:
> I'm a bit uneasy with changing IndexVal outside the if():  when the latter fails,  IndexVal will be incorrect for subsequent users.
> 
> Not sure it's better, but what about something like:
>     Value *OrigIndexVal = ...;
>     if (SDB->findValue(OrigIndexVal)) {
>       IndexVal = OrigIndexVal;
>       ...
>     }
Actually, I don't need the IndexVal any more, but I changed the code.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:3146
@@ +3145,3 @@
+    unsigned GEPWidth = Gep->getType()->getVectorNumElements();
+    MVT VT = MVT::getVectorVT(Index.getValueType().getSimpleVT(), GEPWidth);
+    SmallVector<SDValue, 16> Ops(GEPWidth, Index);
----------------
ab wrote:
> Why not EVT?
The same interface in EVT requires Context. But I changed, the result is the same.


http://reviews.llvm.org/D11121