[PATCH] D75978: [SystemZ] Avoid scalarization of UINT_TO_FP + improve shuffled zext:ing loads.

[Ulrich Weigand via Phabricator via llvm-commits]

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