[PATCH] D30296: v2f32 ops

Jonas Paulsson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 1 04:45:21 PST 2017


jonpa added a comment.

> What is stopping you from implementing SystemZTargetLowering::ReplaceNodeResults to handle these?

The problem I see is with the FP_TO_XINT nodes. The fp->v2i64 are legal, which is correct in the case of v2f64->v2i64. This also means in practice that v2f32 -> v2i64 is legal which it is not, but this is basically ok since there is no vector support for f32. Unfortunately, there is no way to specify that just v2f32->v2iXX should be custom, based on the operand type.

This is a problem there does not seem to be a solution to at the moment - it depends on checking the operand type in this case, while CustomWidenLowerNode() just checks for the result VT.

It might be possible to mark all FP_TO_XINT nodes of all integer vector types as custom, but that would also affect other things, such as cost functions and what not, so I am not sure that would be worth bothering with.

I could apply the patch as it is or move that transformation to ReplaceNodeResults() and maybe just ignore the FP_TO_XINT nodes for now, since fp32 is not a high priority.

> Please can you keep the x86/arm regressions in your patch.

The test regressions are gone because I avoided it with the tighter check - sorry I didn't mention that more clearly.

As I said before, if anything I would like to try to make this a general rule that if a node is going to get widened and then expanded, then it should be expanded if possilbe before type legalization (widening). Otherwise unnecessary scalar operations will be built for the undef elements of the widened vector.


https://reviews.llvm.org/D30296





More information about the llvm-commits mailing list