[llvm] [LLVM] Add `llvm.masked.compress` intrinsic (PR #92289)

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Mon May 20 15:39:00 PDT 2024


topperc wrote:

> > Maybe worth adding ISD::MCOMPRESS to VerifySDNode to ensure the types are correct?
> 
> @RKSimon that's a good idea. As `VerifySDNode` is still quite new, I'm not sure if this is intended behavior or not, but when implementing `MCOMPRESS` in it, I get crashes in my program. Specifically, in `SelectionDAG::getNode()`, there is a step that tries to `FoldConstantArithmetic`, which creates an `MCOMPRESS` node with scalar input. As this is not valid, my verify code crashes. I assume this will happen for many other nodes that require vector inputs.
> 
> I could add a check at the beginning of `FoldConstantArithmetic` and bail, like `CONCAT_VECTORS`
> 
> https://github.com/llvm/llvm-project/blob/84aee95124549c5d13e22053af254e3fcc02bc84/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L6279-L6283
> 
> but that feels like the wrong place/abstraction to do this at. Do you have any suggestions where to put logic to avoid this? In my opinion, the crash in `VerifySDNode` is correct here and the scalar path should never be tried, so we probably want to avoid it.

Why can't we check the types in getNode like we do for most nodes. This node isn't variadic so it doesn't have a different number of operands like BUILD_VECTOR. I'm not sure why BUILD_PAIR is VerifySDNode.

https://github.com/llvm/llvm-project/pull/92289


More information about the llvm-commits mailing list