[PATCH] D87452: [InstCombine] matchRotate - support (uniform) constant rotation amounts (PR46895)

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:2115
+    if (match(L, m_APInt(LC)) && match(R, m_APInt(RC)))
+      if ((*LC + *RC) == Width)
+        return L;
----------------
Probably need to either guard or assert against invalid shift amounts here. Don't want to combine -33 and +1.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:2910
+
+          // We can treat fshr as a fshl by flipping the modulo mount.
+          unsigned ModAmt = Amt->urem(BitWidth);
----------------
mount -> amount


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:2922
+
+          Result = LHS;
+          for (unsigned I = 0; I < (BitWidth - ModAmt); ++I)
----------------
Don't we need to check that the fsh operands are the same, or at least LHS->Provider and RHS->Provider are?


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:2924
+          for (unsigned I = 0; I < (BitWidth - ModAmt); ++I)
+            Result->Provenance[I] = LHS->Provenance[I + ModAmt];
+          for (unsigned I = 0; I < ModAmt; ++I)
----------------
I suspect this is inverted, but maybe I misunderstand the direction of the mapping. It would be good to at least add some tests where the fsh amount is not exactly half the bitwidth.

```
Result = fshl(LHS, RHS, 1)
Result[0] = RHS[31]
Result[1] = LHS[0]
...
Result[31] = LHS[30]
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87452