[PATCH] D114354: [DAG] SimplifyDemandedBits - simplify rotl/rotr to shl/srl

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:17052
   EVT VT = N->getValueType(0);
-  if (N->getOpcode() == ISD::SRL && VT == MVT::i32 && ST->hasV6Ops()) {
-    // Canonicalize (srl (bswap x), 16) to (rotr (bswap x), 16) if the high
----------------
RKSimon wrote:
> dmgreen wrote:
> > craig.topper wrote:
> > > Can this be done in isel pattern? There's an existing top16Zero PatFrag that calls MaskedValueIsZero in ARMInstrThumb2.td
> > That does sound good if it will work. It would be a good way to keep the same pattern working, and I would say a tablegen pattern is preferable to a new node type.
> > 
> > I noticed that llvm.bswap.i16 would no longer generate a rev16, which would be a shame to see. The same thing didn't seem to happen on AArch64 though, it was still fine. I'm not entirely sure what the difference was.
> Sorry - I missed these comments before doing my own version, which handles the bswap_upperzero specific case. 
Thanks. This seems OK. although reusing top16Zero might be a little cleaner.

We probably need patterns for tREV16 and t2REV16 too (and some tests to go with them!) I'll update the existing tests with some more triples. I think the patterns should be the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114354