[PATCH] D137140: [SDAG] Allow scalable vectors in ComputeKnownBits

Paul Walker via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Nov 17 08:09:22 PST 2022


paulwalker-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:2982-2984
+    // TODO: Support zeroinitializer pattern
+    if (Op.getValueType().isScalableVector())
+      break;
----------------
reames wrote:
> paulwalker-arm wrote:
> > I don't believe this is possible because `ISD::VECTOR_SHUFFLE` only supports fixed length vectors.  The zeroinitializer pattern you mention should be covered by `ISD::SPLAT_VECTOR`?
> > 
> > So I guess thus can be an assert like with the `ISD::BUILD_VECTOR` case.  Although both those ISD nodes are baked by a SDNode classes that should have asserts within their constructors and so we should have failed long before we get to this code.
> I'm happy to turn this into an assert if desired.  I see value in making it locally obvious that a code path is dead.  This was not obvious to me.  
> 
> You mention an assert in the constructor.  Glancing at ShuffleVectorSDNode constructor, I do not see such a thing.  Am I missing something obvious?
Now you mention it, I'm looking at our downstream version that contains some asserts we really should get upstream :)


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

https://reviews.llvm.org/D137140



More information about the llvm-commits mailing list