[PATCH] D155299: [AArch64][SVE2] Combine add+lsr to rshrnb for stores

[hassnaaHamdi via Phabricator via llvm-commits]

hassnaa-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:20131
+
+  if (AddValue != 1 << (ShiftValue - 1))
+    return SDValue();
----------------
Hi Mat,
I think that check doesn't consider the case when the AddValue has enabled bits other than the most significant bit.
ex: the ShiftValue is 6, and the AddValue is 33 (100001)b 
Is that correct ? 


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:20131
+
+  if (AddValue != 1 << (ShiftValue - 1))
+    return SDValue();
----------------
hassnaa-arm wrote:
> Hi Mat,
> I think that check doesn't consider the case when the AddValue has enabled bits other than the most significant bit.
> ex: the ShiftValue is 6, and the AddValue is 33 (100001)b 
> Is that correct ? 
Do you handle the case when the destination of the AddOperation has an enabled bit in the same index of the enabled bit in the add Value ?
In that case the Add operation will has side affect, and by combining that case into rshrnb, you cancel the side effect of the AddOperation.
Do you agree on that ? or I'm missing something ??


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155299