[PATCH] D128159: [DAG] Enable scalable vectors handling in computeKnownBits

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3468
   case ISD::ZERO_EXTEND: {
     Known = computeKnownBits(Op.getOperand(0), DemandedElts, Depth + 1);
     Known = Known.zext(BitWidth);
----------------
I'm surprised we need so few bail-outs for all the opcodes for scalable vectors? For example, ISD::INTRINSIC_VOID, etc. call into target functions that may not yet deal with this properly. @paulwalker-arm is right that there was discussion about this during SVE sync calls with @efriedma  and Chris Tetreault and I remember distinctly that we were steered away (for what seemed like sensible reasons) from this approach. It's certainly worth having this discussion again now though. I think part of the problem is that you have to make sure every existing, and new, opcode takes sensible action for scalable vectors. Without a new interface to make it clear that DemandedElts now work with scalable vectors there is possibly a danger of introducing subtle bugs without realising it. At the very least it might be safer to explicitly call out all supported opcodes at the start of the function and bail out for any supported ones, before even getting to the switch statement?


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

https://reviews.llvm.org/D128159