On 24 January 2017 at 19:33, Richard Henderson wrote:
> On 01/24/2017 11:16 AM, Peter Maydell wrote:
>> The CCR.STACKALIGN bit controls whether the CPU is supposed to force
>> 8-alignment of the stack pointer on entry to the exception handler.
>
> 8...
>
>> +/* Align stack pointer if the guest wants that */
>> +if ((env->regs[13] & 4) && (env->v7m.ccr & R_V7M_CCR_STKALIGN_MASK)) {
>
> 4...
>
>> env->regs[13] -= 4;
>
> Not alignment. "&= -4"?
We know SP is always at least a multiple of 4. If it's already
a multiple of 8 then (sp & 4) will be false and we leave sp alone.
Otherwise it's a multiple of 4 but not 8, and subtracting 4
makes it 8 aligned (and we set the saved-XPSR bit to indicate
that we need to undo that on exception exit).
You could maybe rephrase the code to look a bit closer to the
v7M ARM ARM pseudocode, but the way it's written now isn't wrong,
so since this patch is only trying to say "do this if STKALIGN
is set rather than all the time" just adjusting the if conditional
seemed the best thing to me.
(The pseudocode checks for "do we need to align" with
"SP<2> AND forcealign", and does the alignment by
ANDing with a mask constructed with "NOT(ZeroExtend(forcealign:'00',32))".
So we do the check the same way it does, but use a subtract
rather than an AND-NOT. (Since we know that bit 2 must be set
then subtracting 4 and masking that bit to 0 are the same thing.)
thanks
-- PMM
