[PATCH] D53594: [GlobalISel] Introduce G_BUILD_VECTOR and G_CONCAT_VECTOR opcodes

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In https://reviews.llvm.org/D53594#1283016, @qcolombet wrote:

> > I'm not sure how this is any better. If we had an opcode G_BUILD_VECTOR_TRUNC, it would have a superset of the functionality of the G_BUILD_VECTOR. You'd still have this powerful opcode, except now you have potential code duplication in handling the two, as well as a hindered G_BUILD_VECTOR.
> > 
> > Unless you're say that G_BUILD_VECTOR _TRUNC can *only* deal with oversize scalars, in which case it's an odd opcode for a corner case (and I would suggest here that we just deal with the inconvenient types, but I'm not married to the idea).
>
> I was suggesting the later.
>
> Generally speaking, I am not a fan of implicit stuff, thus the proposal (and that's easy to have a isBuildVectorLike or something). That said, I'd like to go back as why we need to do that.
>
> So the problem was that we wanted to represent:
>  a(<4 x s32>) = G_BUILD_VECTOR p(s32), q, r, s
>  b(<4 x s8>) = G_TRUNC a
>
> into
>  b(< 4 x s8>) = G_BUILD_VECTOR p(s32), q, r, s
>
> Why can't we keep the "verbose" representation?
>  I would expect it would be easy to handle that in ISel.


As long as <4 x s32> = the G_BUILD_VECTOR could be legalized I think the worst that will happen is that we just get poor codegen.

> Also, just throwing that here, I haven't really thought about it: should we rethink our legality model to check patterns instead of one instruction at a time?
>  I.e., maybe
> 
>   a(s8) = ...
> 
> 
> is not legal but
> 
>   a(s8) = ...
>   b(<4 x s8>) = build_vector a(s8)
> 
> 
> is.

I think the issue of sequences becoming illegal because of things like copies being introduced is still a deal breaker for that, and it would also introduce a code motion barrier since we can't select sequences spanning blocks. E.g. it wouldn't be safe to move the insn in the predecessor due to other uses.

> This actually ties back to one question that I don't think we answered. Do we legalize on the type (i.e., s8 to s32) or the size and type can be different (i.e., s8(8b) to s8(32b))?
>  It seems to me we are trying to write <4 x s8> = s8(32b), ...., but maybe we want <4 x s8> = s32, ....

I've had a think about what having separate type and container sizes would actually mean. For brevity, let’s call values which have distinct type and container sizes “partial values”. A partial value s8:s32 means that the upper 24 bits of this 32 bit register is always undef. For some operations, it should always be possible to ignore the type size and just use the container size for selection. E.g. non-flag setting G_ADD could select a 32 bit add and satisfy the semantics of an s8 add. But for other operations like shifts, or count-leading-zeros, we can't use a wider operation type, so the entire chain of partial value operations would be restricted to selecting the narrower type, rather than the container. And at that point, the container type may as well not be there. There may be a way around this issue I haven't seen though.

> Anyhow, to be concrete, I am fine with the implicit truncate, we know this is not too bad based on SDISel experience, I want to make sure we consider all our options here.

Sure. Given that we already have this implicit truncating behavior on account of G_MERGE being an omnipotent operation, should we allow the implicit truncation with G_BUILD_VECTOR for now? If we have problems with it later we can revisit the idea of adding a separate opcode/something else.

> Cheers,
> -Quentin


Repository:
  rL LLVM

https://reviews.llvm.org/D53594