Page MenuHomePhabricator

[AArch64] Fold neg(csel(neg(a), b)) to csel(a, neg(b))
ClosedPublic

Authored by dmgreen on Oct 21 2021, 12:54 AM.

Details

Summary

This folds a negation through a csel, which can come up during the lowering of negative abs.

Diff Detail

Event Timeline

dmgreen created this revision.Oct 21 2021, 12:54 AM
dmgreen requested review of this revision.Oct 21 2021, 12:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 21 2021, 12:54 AM

Looks like a useful optimisation @dmgreen! I left a few minor comments ...

llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
14993

Is it worth having a simple helper function like isNegate for all these cases. Perhaps one already exists?

15003

Does this work when both inputs are negated and do we have a test for that?

15009

I understand you're reusing the opcode and operands here, but it took me a while to realise that! Is it worth just writing out what we're doing here fully for clarity, i.e. something a bit like

SDValue Zero = N->getOperand(0);
SDValue N0N = DAG.getNode(ISD::SUB, DL, VT, Zero, N0);
SDValue N1N = DAG.getNode(ISD::SUB, DL, VT, Zero, N1);
This revision was not accepted when it landed; it landed in state Needs Review.Feb 22 2022, 1:59 AM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.

Whoops, that was the wrong differential revision. It should have been https://reviews.llvm.org/D118595