[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