[PATCH] D122703: [AArch64][InstCombine] Fold MLOAD and zero extensions into MLOAD

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:14422-14423
+
+    // If the mask is fully covered by the MLOAD, we don't need to push
+    // a new AND onto the operand
+    EVT EltTy = MemVT.getVectorElementType();
----------------
The masked load's MemVT is not sufficient as you must also consider the load's extension type.

1. As it stand this transform is only safe for zero-extending masked loads...
2. for an any existing masked load the extension can be changed to zero-extending...
3. for a single use sign extending load the extension can be changed to zero-extending.

How far down this path you go depends on the exact use case you need.  If it's just (1) then I think this can be a common DAGCombine. For (2) and (3) it'll depend on whether there are sufficient TLI hooks to ensure you don't create a version a target cannot lower. That said, given you require the input to be an extending masked load perhaps that is not a problem.

One final issue is the handling of the masked load's pass through value.  With the `AND` in place this will be zero-extended but once removed that stops and thus that will need to be considered as well.  My guess is that for today's use case you want to restrict the combine to instances where the pass through value is either undef or zero.


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

https://reviews.llvm.org/D122703