[PATCH] D123781: [AArch64] Add `foldADCToCINC` DAG combine

Paul Walker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 29 07:29:28 PDT 2022


paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

In D123781#3477558 <https://reviews.llvm.org/D123781#3477558>, @Kmeakin wrote:

> In D123781#3475209 <https://reviews.llvm.org/D123781#3475209>, @paulwalker-arm wrote:
>
>> It's not immediately clear to me what the advantage is to using `CINC` verses `ADC`.  Sure some of the tests emit less code but for those cases it just looks like we're not making good use of `xzr`?
>
> It's true that both instructions have the same throughput and latency (on all the microarchitecures I could find the optimisation guides for), but I would argue that `CSINC` is a more natural/canonical way of representing this operation than `ADC`, which could make it easier for future optimisations to recognise it.

Fair enough, but then in D124464 <https://reviews.llvm.org/D124464> you use `getNodeIfExists` to see if there's an existing `ADC` to use in place of `ADCS` which depending on the ordering of combines might mean we don't find a candidate because it's been converted to `CINC`.  That said, perhaps this isn't a realistic issue and it means we avoid the poor `xzr` usage issue so LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123781



More information about the llvm-commits mailing list