[PATCH] D129995: [AArch64] isDesirableToCommuteWithShift - fix UBFX masked shift comment. NFC.

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I think it is called with a shift - it is checking for _another_ shift further in the pattern. So the final pattern is `(((x >> C) & mask) >> C2)` or `(((x >> C) & mask) << C2)`, but it's not checking much about the original `C2` shift (and I'm not sure they are always constant or not). It looks like the original version of the function before https://reviews.llvm.org/D50667 was called with LHS, so the `N = N->getOperand(0).getNode();` was added to compensate for passing the shift instead.

Adding asserts sounds useful. I also noticed that there was a comment on the base isDesirableToCommuteWithShift that says "though its operand,", that should say "through its operand,". And the formatting is off for the inner if here, if you are making changes. It sounds like updating the comment would be useful, but if you do I think it should have both shifts or make it clearer that N is not the original N. Maybe name it LHS instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129995