simon_tatham marked 2 inline comments as done.
simon_tatham added a comment.

In D72934#1829331 <https://reviews.llvm.org/D72934#1829331>, @dmgreen wrote:

> What is the reason that this can't be lowered in tablegen, in the same way as 
> the VMOVimm's are?


In NEON, immediate VBIC is represented as a single MC instruction, which takes 
its immediate operand already encoded into the NEON format (8 data bits, op and 
cmode). That's the same format that `ARMISD::VBICIMM` has encoded the operand 
in after lowering. So you only need one tablegen pattern, which passes the 
immediate through unchanged between the input and output SDNode types.

In MVE, immediate VBIC is represented as four separate MC instructions, for an 
8-bit immediate shifted left by 0, 8, 16 or 24 bits. Each one takes the 
immediate operand in the 'natural' form, i.e. the numerical value that would be 
combined into the vector lane and shown in assembly. For example, 
`MVE_VBICIZ16v4i32` takes an operand such as `0xab0000` which NEON VBIC would 
represent as `0xab | (control bits << 8)`. So the C++ isel code I've written 
has to undo the NEON encoding and turn it back into the 'natural' immediate 
value plus a choice of which MVE opcode to use.

I suppose an alternative would be to rework the MC representation of MVE 
VBIC/VORR so that they look more like the NEON versions. I don't exactly know 
why MVE was done differently in the first place (the commit here has my name on 
it, but it was a team effort). One possibility is that the pseudo-instruction 
reversed forms `vand` and `vorn` might be hard to represent that way, but I 
don't know.

> Do you have any tests for what would be invalid bic values under MVE?

True, I suppose I could provide some immediates that are valid for other 
`VMOVModImmType`s, like `0xabff`, and make sure nothing goes wrong.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:12181
       BVN->isConstantSplat(SplatBits, SplatUndef, SplatBitSize, HasAnyUndefs)) 
{
     if (SplatBitSize <= 64) {
       EVT VbicVT;
----------------
dmgreen wrote:
> This is OK because we are passing OtherModImm to isVMOVModifiedImm, and MVE 
> supports the same patterns as NEON?
Yes: `OtherModImm` only matches values of the form '8-bit number shifted left 
by a multiple of 8 bits', which is just what MVE VBIC and VORR take as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72934



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to