[PATCH] D142288: [X86] Add basic vector handling for ISD::ABDS/ABDU (absolute difference) nodes

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:10193
     if (AbsOp1->getFlags().hasNoSignedWrap() &&
-        TLI.isOperationLegalOrCustom(ISD::ABDS, VT))
-      return DAG.getNode(ISD::ABDS, SDLoc(N), VT, Op0, Op1);
+        TLI.isOperationLegal(ISD::ABDS, VT)) {
+      SDValue ABD = DAG.getNode(ISD::ABDS, DL, VT, Op0, Op1);
----------------
We are seeing an AArch64 regression because of this patch, and the `isOperationLegalOrCustom` -> `isOperationLegal` change seems to be causing this. We are no longer generating a absolute difference and accumulate instruction, which apparently hinges on this ABDS rewrite first. Looks like all sorts of tests are missing for this, but we will add them.

I don't quite understand this change, why "LegalOrCustom" would not be good enough and how that relates to NSW flags. Any insights would be appreciated. I was also surprised to see:

    setOperationAction(ISD::ABDS,             VT, Custom);

in this patch. I.e., the "Custom" surprised me and how that works with the `isOperationLegal` check.
Again, any tips would be welcome while I look a bit more into this. :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142288/new/

https://reviews.llvm.org/D142288