[PATCH] D75978: [SystemZ] Avoid scalarization of UINT_TO_FP + improve shuffled zext:ing loads.
Ulrich Weigand via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 12 08:08:41 PDT 2020
uweigand added a comment.
Not yet looking at the implementation details, but a couple of comments on the overall approach:
- The UINT_TO_FP changes look good to me. The only question I have is whether we should do the same for SINT_TO_FP? (And in either case, this can probably be committed as a separate patch, apart from the VLLEZ changes.)
- I agree that using VLLEZ to implement the zero-extend is a good idea. I'm wondering about the implementation however; it seems odd to combine two quite different manners of emitting that instruction.
First of all, the "canonicalization in DAGCombiner::visitINSERT_VECTOR_ELT()" problem you mention seems to be a problem in general; the code with the two element-inserts could easily arise just from normal middle-end action (not just that new back-end code you added), and we really ought to be able to handle such code in general. So I think we should try to do that, either by overriding the DAGCombine with our own rule (if that's possible), or possibly handling this as a SystemZDAGToDAGISel::Select rule? In any case, I'm sceptical about the new DAG opcode -- it's always better to avoid those for semantics that *can* be described with standard opcodes, since custom opcodes are opaque to all further analysis by the middle-end ...
The other question is how to handle the zero-extend expansion. Logically, a vector zero-extend is fully equivalent to a shuffle intermixing bytes from the original vector with a zero vector. So I think the back-end ought to handle both cases in the same way. Now, we have logic that will intelligently handle shuffles (the whole GeneralShuffle logic). From what I can see, this never emits an UNPACK -- however, it should in theory be able to emit a MERGE, which would later (if one of the inputs is a zero vector) be transformed into an UNPACK via SystemZTargetLowering::combineMERGE.
Given that, maybe the optimal solution to catch all cases would be:
- first, expand vector zero-extend into a shuffle (always)
- second, make sure that the GeneralShuffle logic (and/or combineMERGE) detects the case where an optimal solution is via element inserts into a zero vector (and falls back to UNPACK otherwise)
- finally (as said above), make sure that element inserts into a zero vector get implemented via VLLEZ in all cases where this is possible
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75978/new/
https://reviews.llvm.org/D75978
More information about the llvm-commits
mailing list