[llvm] [LLVM][CodeGen][SVE] Don't combine shifts at the expense of addressing modes. (PR #149873)

[Ricardo Jesus via llvm-commits]

================
@@ -18010,10 +18010,17 @@ bool AArch64TargetLowering::shouldFoldConstantShiftPairToMask(
       unsigned ShlAmt = C2->getZExtValue();
       if (auto ShouldADD = *N->user_begin();
           ShouldADD->getOpcode() == ISD::ADD && ShouldADD->hasOneUse()) {
-        if (auto ShouldLOAD = dyn_cast<LoadSDNode>(*ShouldADD->user_begin())) {
-          unsigned ByteVT = ShouldLOAD->getMemoryVT().getSizeInBits() / 8;
-          if ((1ULL << ShlAmt) == ByteVT &&
-              isIndexedLoadLegal(ISD::PRE_INC, ShouldLOAD->getMemoryVT()))
+        if (auto Load = dyn_cast<LoadSDNode>(*ShouldADD->user_begin())) {
+          TypeSize Size = Load->getMemoryVT().getSizeInBits();
+          // NOTE: +3 to account for bytes->bits transition.
+          if (TypeSize::getFixed(1ULL << (ShlAmt + 3)) == Size &&
+              isIndexedLoadLegal(ISD::PRE_INC, Load->getMemoryVT()))
+            return false;
+
+          unsigned ScalarSize = Load->getMemoryVT().getScalarSizeInBits();
+          // NOTE: +3 to account for bytes->bits transition.
+          if ((1ULL << (ShlAmt + 3)) == ScalarSize &&
+              Load->getValueType(0).isScalableVector())
----------------
rj-jesus wrote:

Should this be using `Load->getMemoryVT()` instead of `Load->getValueType(0)` since the former's what we use for the offsets?

I don't know what you think, but what if this was moved before the original `if` to something like:
```
if (Load->getValueType(0).isScalableVector())
  return (1ULL << (ShlAmt + 3)) == ScalarSize;
```
such that the original code could remain ignorant about scalable types?

It might also be worth having a single declaration of `Load->getMemoryVT()` since there are three such calls in scope.

(I don't have a strong opinion either way - I just thought I'd raise these suggestions for your consideration.)

https://github.com/llvm/llvm-project/pull/149873