[PATCH] D120953: [AArch64][SelectionDAG] Supports unpklo/hi instructions to reduce the number of loads

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11497
       !ISD::isUNINDEXEDLoad(N0.getNode()) ||
-      ((LegalOperations || VT.isVector() ||
+      ((LegalOperations || VT.isFixedLengthVector() ||
         !cast<LoadSDNode>(N0)->isSimple()) &&
----------------
Sorry for the delay but I've been ponding this on and off.  I cannot shake the feeling that any "is vector" check here is artificial and so limiting it to only fixed length vectors also seems artificial.

Personally I think `isVector()` should be removed and any negative effects on code generation being the result of that target's implementation of `isVectorLoadExtDesirable()` likely being at fault.

That said, I understand this might be above and beyond the work you want to carry out so I guess you making the current restriction slightly less artificial is a nice step in the right direction.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11506
     DoXform = ExtendUsesToFormExtLoad(VT, N, N0, ExtOpc, SetCCs, TLI);
-  if (VT.isVector())
+  if (VT.isFixedLengthVector())
     DoXform &= TLI.isVectorLoadExtDesirable(SDValue(N, 0));
----------------
However, the `isVectorLoadExtDesirable()` question will always be pertinent regardless of vector type and so you shouldn't need this change.


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

https://reviews.llvm.org/D120953