[PATCH] D74032: [DAGCombine][ARM] Combine pattern for REV16

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:5664
+    return SDValue();
+  if (!N0->hasOneUse() || !N1->hasOneUse())
+    return SDValue();
----------------
This check seems over-protective. If only one of the 'and' ops has extra uses, the transform is probably still worth doing. It's ok to add a TODO comment here if you want and make that a small follow-up patch after adding more tests to exercise those cases.


================
Comment at: llvm/test/CodeGen/X86/rev16.ll:228
 
 ; TODO: another pattern that we are currently not matching
 ;
----------------
Remove TODO comment - this is working as expected.
It also works with aarch64. The missed transform for arm/thumb seems to be because there's an early target-specific combine that creates "bfi", so we miss the generic pattern matching for bswap because it only runs with !LegalOperations. I'm not sure if/why we need that restriction.


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

https://reviews.llvm.org/D74032