[PATCH] D109963: [AArch64] Split bitmask immediate of bitwise AND operation

[Wilco Dijkstra via Phabricator via llvm-commits]

Wilco1 requested changes to this revision.
Wilco1 added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:2281
+
+  uint64_t Mask = 0xFFFFULL;
+  for (unsigned i = 0; i < 4; i++) {
----------------
Is there no function to check it is a 16-bit immediate? If not, it would be best to add it.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:2290
+  Mask = 0xFFFFULL;
+  uint64_t OrgNImm = ~OrgImm;
+  for (unsigned i = 0; i < 4; i++) {
----------------
This won't correctly handle 32-bit immediates like 0x1234FFFF since the invert sets the top 32 bits.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:2292
+  for (unsigned i = 0; i < 4; i++) {
+    Mask <<= i * 32;
+    // This immediate can be suitable for single MOV instruction.
----------------
Should be * 8.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:2308
+  // to the position of highest bit set.
+  unsigned NewImm1 = (2U << HighestBitSet) - (1U << LowestBitSet);
+  // Creat a mask which is filled with one outside the position of lowest bit
----------------
These need to be 64 bits


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

https://reviews.llvm.org/D109963