[PATCH] D65440: GlobalISel: Add widenScalar for G_UNMERGE_VALUES sources

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked an inline comment as done.
arsenm added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/LegalizerHelper.cpp:1084-1098
+  if (TypeIdx == 1) {
+    Register Cast = MIRBuilder.buildAnyExt(WideTy, SrcReg).getReg(0);
+    MIRBuilder.buildTrunc(MI.getOperand(0), Cast);
+
+    const unsigned DstSize = DstTy.getSizeInBits();
+    unsigned Offset = DstSize;
+    for (unsigned I = 1; I != NumDst; ++I, Offset += DstSize) {
----------------
dsanders wrote:
> This feels more like lower than widenScalar as it replaces the G_UNMERGE_VALUES with G_LSHR+G_TRUNC. I would have expected widenScalar to change something like:
>     %1, %2 = G_UNMERGE_VALUES %0
> into:
>     %5 = G_ANYEXT %0
>     %1, %2, %3, %4 = G_UNMERGE_VALUES %5
> where %3 and %4 are unused. The main limitation with that is that sizeof(%5) would have to be a multiple of sizeof(%1). If that's not true then it would have to lower instead as G_UNMERGE_VALUES wouldn't be able to handle it.
> 
> What was the reason for emitting G_LSHR+G_TRUNC?
I’m not quite sure where to handle the various steps for merge/unmerge. D65447 is essentially the same, except it bitcasts from a vector which seems more like a lower action. Lower however does not have a specified type for it. I thought using the type hint for these primitives is more relevant than the operation itself.

I’m also having trouble deciding whether to handle the source or destination type for some of these. The fact that widen/narrow are distinct actions is something of a complication. Sometimes a wider type needs to be introduced when narrowing, and it’s not clear to me how it should be handled. This is sometimes avoidable by doing something resembling the opposite on the other type index.

I’m also not sure in cases where the types do not evenly divide whether a single legalize step should attempt to deal with it, or if the target legalization rules should have the burden of specifying actions to intermediate types which would be simpler to handle


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65440/new/

https://reviews.llvm.org/D65440