[PATCH] D115739: [SVE][DAGCombiner] Enable ISD::ABDS and ISD::ABDU for SVE.

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:9351
+  if (TLI.isOperationLegalOrCustom(ABDOpcode, VT))
+    return DAG.getNode(ABDOpcode, SDLoc(N), VT, Op0, Op1);
+
----------------
efriedma wrote:
> If I'm understanding correctly, this is just saying that if you know some number of leading sign bits of the operands, abs(a-b) is equivalent to abds(a,b) (and the equivalent for unsigned).
> 
> This would be more clear if you explicitly checked for leading zero/sign bits, instead of implying them from a SIGN_EXTEND/ZERO_EXTEND opcode.
> 
> I'd like to see non-SVE testcases for this.
@efriedma : I've confused myself several times when creating this patch so perhaps I'm still misunderstanding something but I don't think the checks relate to knowing if leading sign bits exists but rather looking for an indication as to how the operands should be interpreted.  By this I mean knowing that a value is positive or negative is not enough to determine which of `ISD::ABDS` or `ISD::ABDU` will produce the same result as a plain `abs(sub())` sequence.

I couldn't see a clear way to write non-SVE test cases.  The nodes have very limited uses and from what I can see they don't have any of the typical legalisation plumbing either.  The existing AArch64 uses don't suffer the same problem with SVE because there's typically a shorter legal vector type available that uses the same element type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115739