[PATCH] D10867: [Codegen] Added intrinsics 'absdiff' and corresponding SDNodes for absolute difference operation

[Shahid]

ashahid added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2699
@@ +2698,3 @@
+/// blend natively.
+SDValue SelectionDAGLegalize::ExpandVSELECT(SDValue Op) {
+  SDLoc DL(Op);
----------------
hfinkel wrote:
> ashahid wrote:
> > hfinkel wrote:
> > > Why are you adding this in this patch?
> > > 
> > In absence of this, instruction selection was hitting "LLVM ERROR: Cannot select: vselect".
> If we're just plain missing code to expand VSELECT, I'd think you should be able to trigger that with IR for some target, and test it, directly. I'd prefer this be a separate commit if possible.
> 
ExpandVselect is present in LegalizeVecctorOps, so triggering just VSELECT works fine.However our case is in fact "Expansion of VSELECT during expansion of ABSDIFF" which is why this error is being reported.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeDAG.cpp:2935
@@ +2934,3 @@
+    Results.push_back(Tmp1);
+    break;
+  }
----------------
jmolloy wrote:
> hfinkel wrote:
> > ashahid wrote:
> > > hfinkel wrote:
> > > > Both ISD::UABSDIFF and ISD::SABSDIFF are the same?
> > > > 
> > > Correct me if I am wrong, I think with respect to expansion here both are same as the signedness of 'absolute diff' operation depends on the 1st 'SUB' operation.
> > Ah, I see what you mean. The difference at the IR level is that SABSDIFF has NSW on the SUBs, and the unsigned version does not. The nice thing is that we can now do this at the SDAG level too..
> > 
> >   SDNodeFlags Flags;
> >   Flags.setNoSignedWrap(true);
> > 
> >   Tmp1 = DAG.getNode(ISD::SUB, dl, VT, Tmp2, Tmp3, &Flags);
> > 
> > (and similar for the other nodes)
> > 
> I think ISD::SETLT should also change to ISD::SETULT depending on signedness.
Oh nice, so NSW and frnds are diving deep :)


Repository:
  rL LLVM

http://reviews.llvm.org/D10867