Hi Ramana,
>On Mon, Sep 9, 2019 at 6:03 PM Wilco Dijkstra <wilco.dijks...@arm.com> wrote:
>>
>> Currently arm_legitimize_address doesn't handle Thumb-2 at all, resulting in
>> inefficient code. Since Thumb-2 supports similar address offsets use the Arm
>> legitimization code for Thumb-2 to get significant codesize and performance
>> gains. SPECINT2006 shows 0.4% gain on Cortex-A57, while SPECFP improves
>> 0.2%.
>
> What were the sort of code size gains ? It did end up piquing my
> curiosity as to how we missed something so basic. For instance ldr
h264ref is 1% smaller, various other benchmarks a few hundred bytes to 1KB
smaller (~0.1%).
> r0, [r0, #-4080] is valid in Arm state but not in Thumb2. Thus if
> there was an illegitimate address given here, would we end up
> producing plus (r0, -4080) ? Yeah a simple testcase doesn't work out.
> Scratching my head a bit , it's late at night.
If the proposed offsets are not legal, GCC tries something different that is
legal, hence there is no requirement to only propose legal splits. In any
case we don't optimize the negative range even on Arm - the offsets are
always split into 4KB ranges (not 8KB as would be possible).
The negative offset splitting doesn't appear to have changed on Thumb-2
so there is apparently a different mechanism that deals with negative offsets.
So only positive offsets are improved with my patch:
int f(int *p) { return p[1025] + p[1026]; }
before:
movw r3, #4104
movw r2, #4100
ldr r2, [r0, r2]
ldr r0, [r0, r3]
add r0, r0, r2
bx lr
after:
add r3, r0, #4096
ldrd r0, r3, [r3, #4]
add r0, r0, r3
bx lr
> Orthogonally it looks like you can clean up the MINUS handling here
> and in legitimate_address_p , I'm not sure what the status of LRA with
> MINUS is either and thus we should now look to clean this up or look
> to turn this on and see what happens. However that's a subject of a
> future patch.
Well there are lots of cases that aren't handled correctly or optimally
yet - I'd copy the way I wrote aarch64_legitimize_address_displacement,
but that's for a future patch indeed.
> For the record, bootstrap with Thumb2 presumably and the testruns were clean
> ?
Yes, at the time I ran them.
Cheers,
Wilco
