[PATCH] D137141: [SDAG] Allow scalable vectors in ComputeNumSignBits

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4385-4386
   case ISD::EXTRACT_VECTOR_ELT: {
+    if (VT.isScalableVector())
+      return 1;
     SDValue InVec = Op.getOperand(0);
----------------
reames wrote:
> paulwalker-arm wrote:
> > This can never be true can it?
> > 
> > More relevant to the other exit paths but is `return 1;` what we want here rather than `break;`?  I asked because the existing scalable vector early exits do the latter (e.g. a couple of lines down) and so I'm wondering if we're loosing information or if the other exit paths are wrong.
> I think this one is dead.  Extract_vector_elt should always be returning a vector.  We have extract_subvector for vector extract from vector.
> 
> As for the 1 vs break thing, both are correct, but 1 is possible non-optimal.  The code below the switch uses compute known bits, so if our compute known bits handles one of these and the sign bits are known, then 1 is unnecessarily conservative.  
> 
> I'll fix both in a follow up commit.
Addressed in 3fb08d1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137141