On 06/11/2018 01:40, Segher Boessenkool wrote:
> Hi Richard,
>
> On Mon, Nov 05, 2018 at 10:09:30AM +0000, Richard Earnshaw (lists) wrote:
>>>>> Shouldn't you be able to do this per function at least?
>>>>
>>>> do what per function? track speculation?
>>>
>>> disable shrink-wrapping only when any speculation was there
>>> (this is about __bultin_speculation_safe_value, no?)
>>
>> Only indirectly. This is about the tracking code that tracks
>> conditional branches and propagates that information through call/return
>> sequences. Shrink wrapping messes with the prologue/epilogue sequences
>> after the speculation tracking pass has run and unknowingly deletes some
>> of the additional code that was previously inserted by the tracking pass.
>
> Do you have an example of this? Shrink-wrapping does not generally
> delete any code.
>
Well it generates new 'light-weight' prologue and epilogue sequences for
the 'shrunk' code path that lack the establishment of the tracker
register and doesn't know how to move the existing sequence to the new
entry sequence.
Consider this (trivial shrink-wrap case).
int f ();
int g (int a)
{
if (a > 3)
return f() + 4;
return a;
}
Without speculation tracking we get (sorry, aarch64, hope you can follow
this):
g:
cmp w0, 3
bgt .L8 // a > 3
ret
.p2align 2
.L8:
stp x29, x30, [sp, -16]!
mov x29, sp
bl f // f()
add w0, w0, 4 // + 4
ldp x29, x30, [sp], 16
ret
With shrink-wrapping and speculation both enabled, we get the following
code:
g:
cmp sp, 0
csetm x15, ne // Establish tracker - OK
cmp w0, 3
bgt .L8
// tracker not updated, speculation state not re-encoded in SP
ret
.p2align 2
.L8:
stp x29, x30, [sp, -16]!
csel x15, x15, xzr, gt // tracker updated for branch
mov x14, sp
mov x29, sp
and x14, x14, x15 // Encode speculation status in SP
mov sp, x14
bl f
add w0, w0, 4
cmp sp, 0 // extract speculation status from call
ldp x29, x30, [sp], 16
csetm x15, ne
mov x14, sp
and x14, x14, x15 // And re-encode it in return sp
mov sp, x14
ret
So although this code executes correctly from an architectural
perspective, if the early return path is taken speclatively, the caller
receives incomplete information about the speculation that has taken place.
And if we disable shrink wrapping, then obviously things work as
originally intended:
g:
cmp sp, 0
stp x29, x30, [sp, -16]!
csetm x15, ne // establish tracker
mov x29, sp
cmp w0, 3
bgt .L5
ldp x29, x30, [sp], 16
csel x15, x15, xzr, le // Update tracker for branch
mov x14, sp
and x14, x14, x15
mov sp, x14 // tracking encoded into SP for return
ret
.p2align 2
.L5:
csel x15, x15, xzr, gt // As above.
mov x14, sp
and x14, x14, x15
mov sp, x14
bl f
add w0, w0, 4
cmp sp, 0
ldp x29, x30, [sp], 16
csetm x15, ne
mov x14, sp
and x14, x14, x15
mov sp, x14
ret
I'm not asking that shrink wrapping be updated to handle all this; in
fact, I'm not sure it's that easy to do as the branch patterns and
simple-return patterns aren't set up to handle this.
R.
>
> Segher
>
