[PATCH][AArch64] implement aarch64 neon load/store instructions class AdvSIMD (lselem)

Tim Northover t.p.northover at gmail.com
Fri Oct 4 08:07:42 PDT 2013


Hi Hao,

> At first, I tried to used them directly in the final instructions. But I
> failed to print out the instructions correctly. I can't print out the
> information about the layout (16b, 8h, 4s, ...)

I think that problem can be solved. You can create a set of
RegisterOperands VPair2s, VPair4h, ... with PrintMethods that put the
correct ".2s", ".4h" and so on in place.

> At last, I changed the solution. I used the same way ARM backend to match
> the ld/st instructions in 2 steps:

I still don't think that's the right solution. These multiple-register
MCInsts are redundant and fragile, and don't really reflect what's
possible in the instruction-set. ARM appears to be trying to move away
from them (see the comment in the definition of the NEONLdStTableEntry
stuct in ARMExpandPseudoInsts.cpp).

I'm also skeptical about the kill/dead/undef flags being attached in
the Expand phase. I know you just copied the functions, but they seem
to assume that if Q0_Q1 is (say) undef then both Q0 and Q1 are. I
can't seem to provoke a failure because of this in ARM, but it looks
dodgy.

> And I think it may be easy to read or do optimization to we show the
> registers separately, if we can ensure the registers are consecutive.

Show them separately where? I don't quite follow this comment. In
MachineInstr dumps, the readability of "Q0_Q1_Q2_Q3<undef>" vs
"Q0<undef>, Q1<undef>, Q2<undef>, Q3<undef>" is moot. We don't
optimise for that and certainly don't use it to decide on an
implementation.

> (2) About the chain.
> I think last time I didn't add the Chain to the operand. So I fix this by
> adding the Chain in AArch64ISelDAGToDAG.cpp. I don't know how to test it,
> but I think this may be fixed.

As far as I can see, yes, though I think this line could do with a
comment that it's updating users of the chain:

+  ReplaceUses(SDValue(N, NumVecs), SDValue(VLd, 1));

> You are right, it's hard to understand. The instructions are load/store
> about multiple N-element structure. I change the name of some definition.
> And also add comment to explain the load/store instructions:

Thanks Hao, comments are always useful.

Cheers.

Tim.



More information about the cfe-commits mailing list