This is an archive of the discontinued LLVM Phabricator instance.

[AArch64] Remove AArch64ISD::NOT, use vnot instead
ClosedPublic

Authored by dmgreen on Oct 25 2020, 2:37 PM.

Details

Summary

vnot (xor -1) should be equivalent to the AArch64 specific AArch64ISD::NOT node, but allow more folding thanks to all the target independent optimizations. Specifically this allows select(icmp ne, x, y) to become cmeq; bsl y, x as opposed to needing to convert the predicate with cmeq; mvn; bsl x, y

Unfortunately there is a regression in a cmtst test, but the code it selected from was already non-canonical, with instcombine preferring to use an eq predicate instead. Plus the more common case of icmp ne is improved.

Diff Detail

Event Timeline

dmgreen created this revision.Oct 25 2020, 2:37 PM
dmgreen requested review of this revision.Oct 25 2020, 2:37 PM

No strong opinion on the cmtst regression - but adding an equivalent canonicalization in DAGCombine might still be worth it if we can generate such a pattern in DAG somehow.

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

Use DAG.getNOT()?

9654

Use DAG.getNOT()?

dmgreen updated this revision to Diff 300912.Oct 27 2020, 1:16 AM

No strong opinion on the cmtst regression - but adding an equivalent canonicalization in DAGCombine might still be worth it if we can generate such a pattern in DAG somehow.

Thanks for the suggestions. I feel like if the pattern was non-canonical then it's unlikely to cause problems. the full pattern is or(and(not(a), b), and(a, c)) for the bsl and not(icmp eq(and(d, e), 0)) for the cmtst, so unfortunately the not's end up simplifying away.

RKSimon accepted this revision.Oct 27 2020, 3:31 AM

cheers LGTM

This revision is now accepted and ready to land.Oct 27 2020, 3:31 AM
fhahn accepted this revision.Oct 27 2020, 3:47 AM

Thanks for the patch, this looks like a good simplification and also should improve codegen in general.

This revision was automatically updated to reflect the committed changes.