[PATCH] D137547: [AArch64][SVE] Use PTRUE instruction for get_active_lane_mask intrinsic if the range is appropriate for predicator constant

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4758-4759
+      unsigned ElementSize = 128 / Op.getValueType().getVectorMinNumElements();
+      unsigned NumActiveElems =
+          Op.getConstantOperandVal(2) - Op.getConstantOperandVal(1);
+      Optional<unsigned> PredPattern =
----------------
This doesn't look correct because `while` can take 64-bit operation. This code can truncate the result in such a way that a ptrue might incorrectly look like a viable replacement.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4763
+      if ((PredPattern != None) &&
+          NumActiveElems <= (MinSVEVectorSize / ElementSize))
+        return getPTrue(DAG, dl, Op.getValueType(), *PredPattern);
----------------
It likely doesn't really matter but given `getSVEPredPatternFromNumElements` takes an unsigned (because it's linked to the interface used by `getVectorMinNumElements`) perhaps it's clean to perform the `<=` check before calling  `getSVEPredPatternFromNumElements`, that way we know the truncation is not losing any precision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137547