PATCH RFA: ARM iWMMXt code can mishandle function return

Richard Earnshaw rearnsha@arm.com
Tue Jan 27 14:30:00 GMT 2004


> When the ARM generates code for iWMMXt, it requires the stack pointer
> to be aligned to an 8-byte boundary.  If the number of regular 4-byte
> ARM registers to be saved on the stack at function entry is odd, that
> might leave the stack pointer misaligned.  In that case, the ARM
> backend will save an extra register, even though it does not have to
> be saved, to maintain the alignment.  This approach is presumably
> used, instead of simply adjusting the stack pointer, because the extra
> register can be included in the register mask for the stmfd and ldmfd
> instructions, thus saving the instructions required to adjust the
> stack pointer.  This code appears in arm_compute_save_reg_mask().
> There is nothing very wrong with any of this.
> 
> However, there is currently a special case in
> output_return_instruction().  That function assumes that if IP appears
> in the register mask, it must be because it was saved when entering a
> function which uses a frame pointer.  In that case, the ARM code will
> save the original value of the stack pointer into IP as the first
> instruction in the function, and then save IP onto the stack.  It is
> then slightly more efficient to restore IP directly into the stack
> pointer with the ldmfd instruction, rather than restoring IP as usual
> and then moving IP to the stack pointer.  This special case is
> implemented in output_return_instruction().  It is also implemented in
> arm_output_epilogue(), but in the latter case it is conditioned on
> frame_pointer_needed, which is safe.  In output_return_instruction(),
> the only condition is ! IS_INTERRUPT (func_type).
> 
> This means that if we write a function which uses an odd number of
> registers, and if the iWMMXt special case then decides to save IP on
> the stack to maintain stack alignment, and if the frame pointer is not
> needed, that output_return_instruction() will do the wrong thing.
> 
> It turns out to be possible to write such a test case.  The iWMMXt
> special case will only save IP if FP is used in the function, so we
> need to use the -mno-apcs-frame option to make FP a non-fixed,
> non-frame-pointer, register.
> 
> This patch fixes the problem.  It also adds a test case.  The test
> case is miscompiled by the current ARM compiler.
> 
> I've run the compilation tests.  I'm currently trying to see if I can
> run the execution tests using the simulator.

This whole area needs a complete rewrite.  A good starting point might be 
to make the compute-save-regs function take an argument (is it for save or 
restore), then the correct list of registers could be generated there and 
then and it wouldn't be necessary to do as much manipulation afterwards.  
However, ...

> 
> OK for mainline if testing passes?
> 

This is OK for Mainline and 3.4 if it causes no regressions.

R.



More information about the Gcc-patches mailing list