RFC: ARM 64-bit shifts in NEON

Hi All,

I'm trying to implement DImode shifts using ARM NEON instructions. This 
wouldn't be difficult in itself, but making it play nice with the 
existing implementation is causing me problems. I'd like a few 
suggestions/pointers/comments to help me get this right, please.

The existing shift mechanisms must be kept, partly because the NEON unit 
is optional, and partly because it does not permit the full range of 
DImode operations, so sometimes it's more efficient to do 64-bit 
operations in core-registers, rather than copy all the values over to 
NEON, do the operation, and move the result back. Which set of patterns 
are used is determined by the register allocator and its costs mechanism.

The late decision means that the patterns may only use the post-reload 
splitter, and so cannot rely on many of the usual passes to sort out 
inefficiencies. In particular, the lack of combine makes it hard to 
detect and optimize extend-and-copy sequences.

So, I've attached two patches. The first is neon-shifts.patch, and does 
most of the work. The second is extendsidi2_neon.patch, and is intended 
to aid moving the shift amount from SImode registers, but doesn't go as 
far as I'd like.

I've not actually tested any of the output code just yet, so there may 
be logic errors, but those are easily fixed later, and what I'm trying 
to get right here is the GCC machine description.

Given this testcase:

    void
    f (long long *a, int b)
    {
      *a = *a << b;
    }

Without any patches, GCC gives this output, using only ARM core 
registers (in thumb2 mode):

    f:
          ldr     r2, [r0, #0]
          ldr     r3, [r0, #4]
          push    {r4, r5, r6}
          rsb     r6, r1, #32
          sub     r4, r1, #32
          lsrs    r6, r2, r6
          lsls    r5, r2, r4
          lsls    r3, r3, r1
          lsls    r1, r2, r1
          orrs    r3, r3, r6
          str     r1, [r0, #0]
          ands    r4, r3, r4, asr #32
          it      cc
          movcc   r4, r5
          str     r4, [r0, #4]
          pop     {r4, r5, r6}
          bx      lr

With just neon-shifts.patch, we get this output, now with NEON shifts:

f:
         fldd    d17, [r0, #0]   @ int
         mov     r2, r1
         movs    r3, #0
         push    {r4, r5}
         fmdrr   d18, r2, r3     @ int
         vshl.i64        d16, d17, d18
         fstd    d16, [r0, #0]   @ int
         pop     {r4, r5}
         bx      lr

As you can see, the shift is much improved, but the shift amount is 
first extended into two SImode registers, and then moved to a NEON 
DImode register, which increases core-register pressure unnecessarily.

With both patches, we now get this:

f:
         fldd    d17, [r0, #0]   @ int
         vdup.32 d16, r1
         vshr.u64        d16, d16, #32   <-- still unnecessary
         vshl.i64        d16, d17, d16
         fstd    d16, [r0, #0]   @ int
         bx      lr

Now the value is copied and then extended. I have chosen to use vdup.32 
instead of vmov.i32 because the latter can only target half the DImode 
registers. The right shift is necessary for a general zero-extend, but 
is not useful in this case as only the bottom 8 bits are interesting, 
and vdup has already done the right thing.

Note that the examples I've given are for left shifts. Right shifts are 
also implemented, but are a little more complicated (in the 
shift-by-register case) because the shift must be implemented as a left 
shift by a negative amount, and so an unspec is used to prevent the 
compiler doing anything 'clever'. Apart from an extra negation, the end 
result is much the same, but the patterns look different.

All this is a nice improvement, but I'm not happy:

1. The post-reload split means that I've had to add a clobber for CC to 
all the patterns, even though only some of them really need it. I think 
I've convinced myself that this is ok because it doesn't matter before 
scheduling, and after splitting the clobbers are only retained if 
they're really needed, but it still feels wrong.

2. The extend optimization is fine for general case extends, but it can 
be improved for the shift-amount case because we actually only need the 
bottom 8 bits, as indicated above. The problem is that there's no 
obvious way to achieve this:
    - there's no combine pass after this point, so a pattern that 
recognises and re-splits the extend, move and shift can't be used.
    - I don't believe there can be a pattern that uses SImode for the 
shift amount because the value needs to be in a DImode register 
eventually, and that means one needs to have been allocated before it 
gets split, and that means the extend needs to be separate.

3. The type of the shift-amount is determined by the type used in the 
ashldi3 pattern, and that uses SImode. This is fine for values already 
in SImode registers (probably the common case), but means that values 
already in DImode registers will have to get truncated and then 
re-extended, and this is not an operation that can generally be 
optimized away once introduced.
    - I've considered using a DImode shift-amount for the ashldi3 
pattern, and that would solve this problem - extend and truncate *can* 
be optimized away, but since it doesn't get split until post reload, the 
register allocator would already have allocated two SImode registers 
before we have any chance to make it go away.

4. I'm not sure, but I think the general-case shift in core registers is 
sufficiently long-winded that it might be worthwhile completely 
discarding that option (i.e. it might cheaper to just always use neon 
shifts, when neon is available, of course). I'd keep the 
shift-by-constant-amount variants though. Does anybody have any comments 
on that?

5. The left and right shift patterns couldn't be unified because I 
couldn't find a way to do match_operand with unspecs, and anyway, the 
patterns are a slightly different shape.

6. Same with the logical and arithmetic right shifts; I couldn't find a 
way to unify those patterns either, even though the only difference is 
the unspec index number.

Any help would be appreciated. I've probably implemented this backwards, 
or something ...

Thanks a lot

Andrew

RFC: ARM 64-bit shifts in NEON

Commit Message

Comments

Patch