| Message ID | 16928373.Xl9ooNoE5W@polaris |
|---|---|
| State | New |
| Headers | show |
On 22/12/13 17:42, Eric Botcazou wrote: > Hi, > > this is the last issue with nested APCS frames according to our testing. When > the IP register needs to be preserved on entry and r3 isn't free and there are > no arguments to push, the prologue creates a slot above the frame, so various > internal offsets need to be adjusted. One has been missed, leading to: > > sub ip, fp, #20 > fldmfdd ip!, {d8} > sub sp, fp, #16 > ldmfd sp, {r3, fp, sp, pc} > > in the epilogue of the nested frame. That's wrong because the difference > between the 2 immediates must be equal to the size of the saved FP registers. > > Tested on ARM/VxWorks (where it fixes several ACATS tests at -O2) and > ARM/EABI, OK for the mainline? > > > 2013-12-22 Eric Botcazou <ebotcazou@adacore.com> > > * config/arm/arm.c (arm_get_frame_offsets): Revamp long lines. > (arm_expand_epilogue_apcs_frame): Take into account the number of bytes > used to save the static chain register in the computation of the offset > from which the FP registers need to be restored. > > > 2013-12-22 Eric Botcazou <ebotcazou@adacore.com> > > * gcc.target/arm/neon-nested-apcs.c: New test. > > OK, modulo the 2 nits below. > /* In Thumb mode this is incorrect, but never used. */ > - offsets->frame = offsets->saved_args + (frame_pointer_needed ? 4 : 0) + > - arm_compute_static_chain_stack_bytes(); > + offsets->frame > + = offsets->saved_args > + + arm_compute_static_chain_stack_bytes () > + + (frame_pointer_needed ? 4 : 0); There should be brackets around the entire expression when split across mulitple lines, so that auto-indentation is correctly preserved: ie offsets->frame = (expression_in_backets_when - over_mulitple_lines); > /* Find the offset of the floating-point save area in the frame. */ > - floats_from_frame = offsets->saved_args - offsets->frame; > + floats_from_frame > + = offsets->saved_args > + + arm_compute_static_chain_stack_bytes () > + - offsets->frame; > Same here. R.
Hi, > > 2013-12-22 Eric Botcazou <ebotcazou@adacore.com> > > * gcc.target/arm/neon-nested-apcs.c: New test. > This new test fails when GCC is configured as target arm-none-linux-gnueabihf and --with-float=hard: tools/arm-none-linux-gnueabihf/bin/ld: error: ./neon-nested-apcs.exe uses VFP register arguments, /tmp/cc9LLqES.o does not tools/arm-none-linux-gnueabihf/bin/ld: failed to merge target specific data of file /tmp/cc9LLqES.o collect2: error: ld returned 1 exit status This is because startup and library code is compiled in hardfp mode, and is incompatible with the -mfloat-abi=softfp forced in the test. Christophe.
> This new test fails when GCC is configured as target > arm-none-linux-gnueabihf and --with-float=hard: > tools/arm-none-linux-gnueabihf/bin/ld: error: ./neon-nested-apcs.exe > uses VFP register arguments, /tmp/cc9LLqES.o does not > tools/arm-none-linux-gnueabihf/bin/ld: failed to merge target specific > data of file /tmp/cc9LLqES.o > collect2: error: ld returned 1 exit status Is it sufficient to static-ify 'bar' and 'foo'? If so, please do that. > This is because startup and library code is compiled in hardfp mode, > and is incompatible with the -mfloat-abi=softfp forced in the test. Other tests in the gcc.target/arm directory do the same though.
On 10/01/14 09:46, Eric Botcazou wrote: >> This new test fails when GCC is configured as target >> arm-none-linux-gnueabihf and --with-float=hard: >> tools/arm-none-linux-gnueabihf/bin/ld: error: ./neon-nested-apcs.exe >> uses VFP register arguments, /tmp/cc9LLqES.o does not >> tools/arm-none-linux-gnueabihf/bin/ld: failed to merge target specific >> data of file /tmp/cc9LLqES.o >> collect2: error: ld returned 1 exit status > > Is it sufficient to static-ify 'bar' and 'foo'? If so, please do that. > >> This is because startup and library code is compiled in hardfp mode, >> and is incompatible with the -mfloat-abi=softfp forced in the test. > > Other tests in the gcc.target/arm directory do the same though. > Some tests can do it because they don't produce an executable. R.
> Some tests can do it because they don't produce an executable.
OK, there is some related dg-skip-if magic in 20090811-1.c, maybe Christophe
can try that too.
On 10/01/14 11:48, Eric Botcazou wrote: >> Some tests can do it because they don't produce an executable. > > OK, there is some related dg-skip-if magic in 20090811-1.c, maybe Christophe > can try that too. > Can you not use (or add something similar to) { dg-add-options arm_neon }? R.
> Can you not use (or add something similar to) { dg-add-options arm_neon }?
Probably, yes, something like:
/* { dg-do run } */
/* { dg-require-effective-target arm_neon_hw } */
/* { dg-options "-fno-omit-frame-pointer -mapcs-frame -O" } */
/* { dg-add-options arm_neon } */
On 10/01/14 12:28, Eric Botcazou wrote: >> Can you not use (or add something similar to) { dg-add-options arm_neon }? > > Probably, yes, something like: > > /* { dg-do run } */ > /* { dg-require-effective-target arm_neon_hw } */ > /* { dg-options "-fno-omit-frame-pointer -mapcs-frame -O" } */ > /* { dg-add-options arm_neon } */ > Yes, something like that. R.
> > /* { dg-do run } */ > > /* { dg-require-effective-target arm_neon_hw } */ > > /* { dg-options "-fno-omit-frame-pointer -mapcs-frame -O" } */ > > /* { dg-add-options arm_neon } */ > > Yes, something like that. OK, installed after checking that this doesn't change the test, thanks.
Index: config/arm/arm.c =================================================================== --- config/arm/arm.c (revision 206161) +++ config/arm/arm.c (working copy) @@ -20316,8 +20316,10 @@ arm_get_frame_offsets (void) offsets->saved_args = crtl->args.pretend_args_size; /* In Thumb mode this is incorrect, but never used. */ - offsets->frame = offsets->saved_args + (frame_pointer_needed ? 4 : 0) + - arm_compute_static_chain_stack_bytes(); + offsets->frame + = offsets->saved_args + + arm_compute_static_chain_stack_bytes () + + (frame_pointer_needed ? 4 : 0); if (TARGET_32BIT) { @@ -20357,9 +20359,10 @@ arm_get_frame_offsets (void) } /* Saved registers include the stack frame. */ - offsets->saved_regs = offsets->saved_args + saved + - arm_compute_static_chain_stack_bytes(); + offsets->saved_regs + = offsets->saved_args + arm_compute_static_chain_stack_bytes () + saved; offsets->soft_frame = offsets->saved_regs + CALLER_INTERWORKING_SLOT_SIZE; + /* A leaf function does not need any stack alignment if it has nothing on the stack. */ if (leaf && frame_size == 0 @@ -27044,7 +27047,10 @@ arm_expand_epilogue_apcs_frame (bool rea saved_regs_mask = offsets->saved_regs_mask; /* Find the offset of the floating-point save area in the frame. */ - floats_from_frame = offsets->saved_args - offsets->frame; + floats_from_frame + = offsets->saved_args + + arm_compute_static_chain_stack_bytes () + - offsets->frame; /* Compute how many core registers saved and how far away the floats are. */ for (i = 0; i <= LAST_ARM_REGNUM; i++)
Hi, this is the last issue with nested APCS frames according to our testing. When the IP register needs to be preserved on entry and r3 isn't free and there are no arguments to push, the prologue creates a slot above the frame, so various internal offsets need to be adjusted. One has been missed, leading to: sub ip, fp, #20 fldmfdd ip!, {d8} sub sp, fp, #16 ldmfd sp, {r3, fp, sp, pc} in the epilogue of the nested frame. That's wrong because the difference between the 2 immediates must be equal to the size of the saved FP registers. Tested on ARM/VxWorks (where it fixes several ACATS tests at -O2) and ARM/EABI, OK for the mainline? 2013-12-22 Eric Botcazou <ebotcazou@adacore.com> * config/arm/arm.c (arm_get_frame_offsets): Revamp long lines. (arm_expand_epilogue_apcs_frame): Take into account the number of bytes used to save the static chain register in the computation of the offset from which the FP registers need to be restored. 2013-12-22 Eric Botcazou <ebotcazou@adacore.com> * gcc.target/arm/neon-nested-apcs.c: New test.