The implementation just extends the vector to a larger element type, and extracts from that. Not fancy, but generates reasonable code.
While I'm here, fix warning from computeKnown bits.
efriedma on Sep 14 2020, 4:18 PM.Authored by
Rebase and clang-format.
I looked into handling this in LegalizeVectorOps, but that doesn't really work out: we actually legalize EXTRACT_VECTOR_ELT nodes in LegalizeDAG. And I don't want to do this in LegalizeDAG: LegalizeDAG can't handle arbitrary illegal vector operations.
There might be some way to sort out this mess, but it isn't trivial.
Perhaps I'm missing something but I've created D89950 to show what I mentioned in my previous comment. To me it's preferable to have this as target independent code as it seems a common enough solution. I know it results in "Pomote" having multiple meanings but that boat has sailed because different nodes have already chosen different meanings and I think it's pretty clear from the old and new VTs what's being asked for.
I'm worried the usage of ANY_EXTEND and TRUNCATE in D89950 is going to cause a mess in the future. LegalizeDAG doesn't know how to legalize most vector operations; LegalizeVectorOps handles them. If someone tries to use "Promote" on a target where those operations aren't legal, it'll break. And then someone will try to "fix" LegalizeDAG, and cause other problems.
Note this is specific to the vector operations we legalize in LegalizeDAG; so essentially, INSERT_VECTOR_ELT, EXTRACT_VECTOR_ELT, and VECTOR_SHUFFLE.
I just don't buy this argument. You're saying that we must force all target's to perform custom lowering for promotable illegal operations on i1 vectors because somebody in the future might try to "fix" LegalizeDAG when they should implement custom lowering for extend/truncate operations. This is the situation Target/AArch64 is in and we never considered "fixing" LegalizeDAG so I don't see why others wouldn't just follow our example.
I've already pointed out:
setOperationAction(ISD::FADD, MVT::v4f16, Promote); AddPromotedToType(ISD::FADD, MVT::v4f16, MVT::v4f32);
that exists today and is happy to emit ISD::FP_EXTEND. We mirrored this to support i1 based int->fp operations, although perhaps that's a case of two wrongs not making a right :)
Sure we can teach LegalizeVectorOps LegalizeDAG's PromoteNode abilities but that feels like wasted effort considering it just works. My understand of the two legalisers is that LegalizeVectorOps might emit scalar operations that require further legalisation, which is a situation that doesn't apply here considering it's vector in vector out.
So ultimately I don't know what to do here. There's nothing functionally wrong with the patch (other than a couple of minor suggestions) so I guess I'll begrudgingly accept the patch but I just think you're causing artificial restrictions that others have not felt the need to apply and thus hope you'll reconsider.