[VP] Condition in vp.select|merge not a VP mask

[simoll]

Summary

vp.select|merge both select lanes based on a condition mask. Unlike other VP intrinsics the lanes are defined where the condition mask is false. Hence, the condition mask in vp.select|mask is not a mask in the sense of VP intrinsics.
By doing not treating the condition mask specially, vp.select becomes the canonical VP translation of the select instruction.

[frasercrmck]

Makes sense to me: it's not a mask. I was looking around to see what knock-on effects this has, and good news is that I think this fixes a bug we're not testing where an all-zeros "mask" will DAGCombine to undef (visitVPOp). Could you maybe pre-commit a test for that and show this patch fixing it?

On that note: I don't know if this is truly a child of D105283. Can't we merge it right away?

[simoll]

In D118981#3305224, @frasercrmck wrote:

Makes sense to me: it's not a mask. I was looking around to see what knock-on effects this has, and good news is that I think this fixes a bug we're not testing where an all-zeros "mask" will DAGCombine to undef (visitVPOp). Could you maybe pre-commit a test for that and show this patch fixing it?

On that note: I don't know if this is truly a child of D105283. Can't we merge it right away?

I'll commit an XFAIL test for VE and update with this patch to a passing test. D105283 only depends on this patch (no other way to add dependence edges) - we can merge this.

[simoll]

In D118981#3305224, @frasercrmck wrote:

Makes sense to me: it's not a mask. I was looking around to see what knock-on effects this has, and good news is that I think this fixes a bug we're not testing where an all-zeros "mask" will DAGCombine to undef (visitVPOp). Could you maybe pre-commit a test for that and show this patch fixing it?

On that note: I don't know if this is truly a child of D105283. Can't we merge it right away?

Actually, DAGCombine won't do that because it only folds bvp inary operators mem ops and reduction. So, also nothing to test.

[frasercrmck]

In D118981#3311747, @simoll wrote:

In D118981#3305224, @frasercrmck wrote:

Makes sense to me: it's not a mask. I was looking around to see what knock-on effects this has, and good news is that I think this fixes a bug we're not testing where an all-zeros "mask" will DAGCombine to undef (visitVPOp). Could you maybe pre-commit a test for that and show this patch fixing it?

On that note: I don't know if this is truly a child of D105283. Can't we merge it right away?

Actually, DAGCombine won't do that because it only folds bvp inary operators mem ops and reduction. So, also nothing to test.

Yes, you're right, sorry. visitVPOp is more careful than I thought.

LGTM.

llvm/include/llvm/IR/VPIntrinsics.def

HELPER_REGISTER_REDUCTION_SEQ_VP(vp_reduce_fmul, VP_REDUCE_FMUL,
                                  experimental_vector_reduce_fmul)
 
 #undef HELPER_REGISTER_REDUCTION_SEQ_VP
 
 ///// } Reduction
 
 ///// Shuffles {
 
-// llvm.vp.select(mask,on_true,on_false,vlen)
-BEGIN_REGISTER_VP(vp_select, 0, 3, VP_SELECT, -1)
+// llvm.vp.select(cond,on_true,on_false,vlen)
+BEGIN_REGISTER_VP(vp_select, None, 3, VP_SELECT, -1)
 VP_PROPERTY_FUNCTIONAL_OPC(Select)
 END_REGISTER_VP(vp_select, VP_SELECT)
 
-// llvm.vp.merge(mask,on_true,on_false,pivot)
-BEGIN_REGISTER_VP(vp_merge, 0, 3, VP_MERGE, -1)
+// llvm.vp.merge(cond,on_true,on_false,pivot)
+BEGIN_REGISTER_VP(vp_merge, None, 3, VP_MERGE, -1)
 END_REGISTER_VP(vp_merge, VP_MERGE)
 
 BEGIN_REGISTER_VP(experimental_vp_splice, 3, 5, EXPERIMENTAL_VP_SPLICE, -1)
 END_REGISTER_VP(experimental_vp_splice, EXPERIMENTAL_VP_SPLICE)
 
 ///// } Shuffles
 
 #undef BEGIN_REGISTER_VP