[PATCH] D124908: [SVE] Optimize new cases for lowerConvertToSVBool

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:4196
+  // which don't use a ptrue intrinsic.
+  if (isAllActivePredicate(DAG, InOp))
+    return Reinterpret;
----------------
In this context I don't believe `isAllActivePredicate` is safe to use. `isAllActivePredicate` will look through some `AArch64ISD::REINTERPRET_CAST` nodes because they promise not to use the "invisible" lanes they produce.  For this code path you're asking the opposite question of "are the invisible lanes known to be zero".  Using `isAllActivePredicate` in this way will allow the invalid transformation `toSVBool(reinterp_nxv2(ptrue_s)) -> ptrue_s`.  I think you instead need to check `InOp` is specifically the "splat of 1" node (i.e. `ISD::isConstantSplatVectorAllOnes(...)`).

If you agree please add a negative test so that we can validate the current behaviour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124908