[PATCH] D135852: [AArch64] Enhance bit-field-positioning op matcher to see through "any_extend" for pattern "and(any_extend(shl(val, N)), shifted-mask)"

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2573
+    // For pattern "and(shl(val, N), shifted-mask)", 'Src' is set to 'val'.
+    Src = AndOp0.getOperand(0);
+  } else if (VT == MVT::i64 && AndOp0.getOpcode() == ISD::ANY_EXTEND &&
----------------
Can we create a new variable for the temporary SDValue?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2577
+                                   ShlImm)) {
+    // For pattern "and(any_extend(shl(val, N)), shifted-mask)"
+
----------------
Add an assert that the type of ShlVal is i32?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2582
+
+    // Widdens val to MVT::i64, and assigns it to 'Src'.
+    Src = Widen(CurDAG, ShlVal.getOperand(0));
----------------
-> Widens


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2600
+  // clamp Width according to source bit width.
+  assert(VT.isSimple() && VT.isScalarInteger() &&
+         "VT should be a simple scalar type");
----------------
I think this will always be a MVT::i64 or MVT::i32?


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp:2602
+         "VT should be a simple scalar type");
+  Width = std::min(Width, (int)VT.getSizeInBits());
+
----------------
Do we have any tests where this applies?


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

https://reviews.llvm.org/D135852